Code review init, changemanager, ...

[ehhartmann]

• init
• changemanager (removed)
• config
• config tests
• schema (do towards the end of review)
• parsing
• parsing tests
• reaction -> recipe and plugins.py
• recipe tests
• Tasks
• utils

[ehhartmann]

Review init file:
https://github.com/hits-mbm-dev/kimmdy/blob/8ac75e78cfe6d298ea578c4910bd0ae0765c6cea/src/kimmdy/__init__.py

• typing is done, no functions or classes
• code for reaction and parameterization plugins is mostly duplicate, could make a discovered_plugins dict for reaction and parameterization and load plugins in a loop
• maybe move the entry_point.load() to the runmanager and only load used entry_points? Start up of kimmdy is currently quite slow

[ehhartmann]

Review changemanager:

• maybe rename
• refactor parts of it to runmanager._run_recipe to avoid passing booleans back and iterating through the same loop
• remove focus in modify_top

[ehhartmann]

Review utils.py:

• possible duplicate with misc_helper.py
• get_gmx_dir() would not work if a different gmx prefix is used? But no access to prefix at early stages of the run where it is used
• maybe a more explicit and systematic discovery of the gmx state would be beneficial for start up
• create file for functions that may be helpful when creating reaction plugins (maybe -> plugins.py or new reaction_building_blocks.py
• maybe split get_atominfo_from plumedid into atoms from plumedid and atominfo from atoms (doesn't topology have methods for that?)
• homolysis should use runmgr.top and pass it on to the functions in utils.py
• frozensets are unnecessary and could just be tuples? Why conversion between list and frozenset?
• the lookup functions could also be an operation on all plumedids, would be more efficient
• consistent naming for Topology, TopologyDict, path to topology file? In general consistent parameter docstrings for the same parameters?
• explicit type in variables unnecessary?
• no logging at all but I would leave it to the code that calls these functions

[ehhartmann]

finish after homolysis is rewritten

[ehhartmann]

Review Task.py

• docstrings focus on typical flow of kimmdy and where the information is typically coming from, may not be very general
• very sparse logging -> general logging of tasks here?

[ehhartmann]

Testing:

• put config in tox.ini
• reference to tests of plugins
• skip plugin tests if plugins are not in kimmdy reaction_plugins / parameterization_plugins
• skip test if dependency (grappa) is not available
• decouple tests (duplicate for integration test??)
• skip tests that need gromacs if there is none?

[ehhartmann]

parsing:

• TopologyDict should be somewhere else, maybe in the topology dir?
• remove xml parsing, not used anymore,
• remove read_rtp