Refactor config.py

[blakegong]

I was quite puzzled by the Java-ish pattern in config.py, and also the pain to read/write ini files for my non-Windows brain.

So, decided to refactor the module. I believe the new config.get(key) & config.set(key, value) pattern would be very easy to refactor in the future, as it's just one API. 5 hours have been spent for this PR, so having scattered around API is really not that ideal for refactoring. Let me know what you think 😄

Upon first calibration, this will create a new config.json file. You can manually migrate config.ini to that json, but I'd think that recalibrating would be much faster.

Oh and I've created nestris_ocr folder. I think in the future code should all be moved into there. Let me know if you think it should be nestris_ocr or nes_tris_ocr.

[alex-ong]

I believe the pythonic way would be to use config['item'] rather than config.get('item')? And looks like you flatten/unflatten; what is better from using config, config['section1']['key'] or config['section1.key']?

And then you can use __getitem__ / __setitem__ in config class to implement it? I think it wouldn't work for config['section1']['section2'] if you're using __getitem__.

Just wondering; i'm happy as it is but i prefer the config['section']['key'] approach or at least config['section.key'] instead of config.get() / config.set().

Another thing I noticed was partials everywhere. I prefer lambda x: function('fixed.value', x) vs partial(function, 'fixed.value') I did some research and looks like partials is the correct way, with lambdas being a blight upon python. TIL

[blakegong]

@alex-ong yes that's very right! Pythonic way is definitely doing things in dictionary.

I've missed a line of code in my late night editing, so the intention was to write the get() function like:

def get(self, key):
    if key not in CONFIG_DEFAULTS:
        raise KeyError("Invalid key")

    return self.data.get(key, CONFIG_DEFAULTS[key])

Notice the fallback to CONFIG_DEFAULTS. It's a safety net for when people accidentally removed any attribute out of the cached config.json.

With dictionary notation of config['foo']['bar'], this is still doable, but the effort would soar....

I think I'd just do config['foo.bar'] for now 😄 Easy change, and we'd both be happy.

The partial is a sad result of how the UI code is written right now. But no big issue. That can be fixed very soon. I just didn't have bandwidth to deal with anything extra at 3am last night.

[alex-ong]

sounds great! Should i merge now and then do anotehr PR for getitem setitem or wait for that?

[blakegong]

If you are not updating much code, just let this stay open please. I'd hope to address that.

But if you do need to update anything, just let me know and merge this. I can issue another PR afterwards too.

[alex-ong]

Looks good! merging.