Config dict

[uhager]

No description provided.

[FussyDuck]

All committers have signed the CLA.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.00000% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.01%. Comparing base (c0b6ccb) to head (74bb56b).

Files with missing lines	Patch %	Lines
ifsbench/config_mixin.py	93.75%	4 Missing ⚠️
ifsbench/data/namelisthandler.py	97.77%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #10      +/-   ##
==========================================
+ Coverage   78.15%   79.01%   +0.86%     
==========================================
  Files          22       23       +1     
  Lines        1927     2035     +108     
==========================================
+ Hits         1506     1608     +102     
- Misses        421      427       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[johannesbulin]

I'm quite happy with the idea of using a mixin. The code is (as usual) well written, simple and clean.

I have a few concerns, mainly regarding the use of type hints:

• As you can see in the checks, the currently used type hints don't work for all Python versions (major changes were added in Python 3.9, some smaller later). I don't know whether we can make it work for all versions.
• In my opinion, type hints are hints - and are usually not enforced.

This is probably not a blocking issue. Unless I am mistaken, we could simply check the types at (de-)serialisation time.

One open question: How would you deserialise an object (i.e. from YAML to a Python object). I couldn't find any references to the Python objects class/module yet and it would also need some kind of from_dict class function to recover an object.

[johannesbulin]

Just a comment for later (I now that this is a draft). Most output in ifsbench is done via the debug/info/warning/.... functions in ifsbench.logging.

[uhager]

This was just a debug output I forgot. Removed now

[johannesbulin]

Type comparisons might be dangerous here, if we work with subclasses (for example if a class stores multiple DataHandler objects in a list).

[uhager]

Changed to isinstance

[johannesbulin]

Should probably be given a less generic name like _mixin_config. Otherwise this might clash with classes that have their own _config attribute (which is probably not that rare).

[uhager]

Done.

[mlange05]

@uhager First of all, many thanks for looking into this, and even though it's still a draft, I think this is a very promising start for a complex design issue. I've not looked through this in detail, but the initial discussion is already super interesting and valuable. So, thanks for your initiative! 🙏

@uhager and @johannesbulin I agree on both accounts that the Mixin approach is great, and that using type-info for the necessary introspection are very neatly done.

On @johannesbulin point about type-safety and automated checking, I would like to point out that there is also the option to use Pydantic: https://docs.pydantic.dev/latest/
This is a library that performs strong type checking against Python type hints, and allows various levels of control over strictness and automatic type-coaxing. We use it in one of the lower layers of Loki, and we have Python version support for Python 3.8-3.12 (3.13 is missing for other reasons). What I also find convenient about Pydantic is the use of a BaseModel abstraction that allows you to specify the types as class properties, rather than type-hints to the constructor. This comes with neat features like "validators" that can be used to customise our own "auto-conversion" of types, if we need to.

And lastly, I'm open to either solution, but as type-safety and introspection were raise, I thought I'd throw out some pointers.

[uhager]

One open question: How would you deserialise an object (i.e. from YAML to a Python object). I couldn't find any references to the Python objects class/module yet and it would also need some kind of from_dict class function to recover an object.

I think that should be a separate function / class with implementations for yaml, json, or whatever we want to support. That function will parse the yaml, create the config dicts, and call the build functions in the classes that need to be created

[johannesbulin]

One open question: How would you deserialise an object (i.e. from YAML to a Python object). I couldn't find any references to the Python objects class/module yet and it would also need some kind of from_dict class function to recover an object.

I think that should be a separate function / class with implementations for yaml, json, or whatever we want to support. That function will parse the yaml, create the config dicts, and call the build functions in the classes that need to be created

Fine with the separate function to start the whole (de-)serialising process. Still, if you have a class like this

class SomeBenchmark(ConfigMixin):
   def __init__(self, data_handler):
       # data_handler is list of DataHandler objects.
       self._data_handler = list(data_handler)

you would have ConfigMixin objects (in data_handler) encapsulated in another ConfigMixin object (SomeBenchmark instance). How would you serialise this? If I understand this correctly, config_format in SomeBenchmark should also call config_format in the data_handler objects.
And at this point you would also have to store information on classes/modules.

[uhager]

On @johannesbulin point about type-safety and automated checking, I would like to point out that there is also the option to use Pydantic:

Looked into that, looks promising. I have never used pydantic, so to see what's possible I made a prototype implementation: #13

That approach is a lot cleaner than this one, but may end up less flexible. So far it's looking good though.