MAINT: Upgrade ALL network configuration file formats (except for tests)
Currently, the newest version of the network configuration files are the “hierarchical JSON” format, which are required for any networks loaded using the GUI. We should complete the “migration” of all network configuration files we actually expect users to use into this format, and either remove or archive all “legacy” config files, including the “flat JSON” format and the original “param” format (except for use in testing).
Part of why this is important is that if a user is using the GUI, then all JSON network config files they potentially interact with, such as https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/param/default.json , EXCEPT for https://github.com/jonescompneurolab/hnn-core/tree/master/hnn_core/tests/assets , should be using the latest format.
Once this is done, we should also include a config_version attribute into the format in order to track version differences of the config files themselves.
hey @asoplata ,i have understood the task there is one question in my mind that should i archive all that legacy files in another directory for historical use or delete them .i am thinking to transfer them in another directory and update the .gitignore or documentation that they are not for regular use
one more thing as we are migrating into hierarchial json format ,does the gui script is able to handle the nested structure or we need to update that also
Hello Dikshant! Welcome, and thank you very much for your interest in contributing to HNN-Core's development! We're very happy to have you here :)
Firstly, our Contribution Guide can be found here: https://jonescompneurolab.github.io/hnn-core/stable/contributing.html . Judging from your Python experience, you should already be pretty familiar with how we do things. We do rebase+squash instead of merges.
Secondly, our Code of Conduct can be found here: https://github.com/jonescompneurolab/hnn-core/blob/master/CODE_OF_CONDUCT.md .
Lastly, if you are interested in contributing to development, we strongly recommend that you start with an existing Issue that has the good first issue tag. Some examples are #954, #917, #966, and #967. Unfortunately, this current issue is not a good fit for a new developer, since there's certain complexities for how to do this that I didn't include in the initial issue description.
hii @asoplata thanks for guiding me i will start with the good first issue first then will move forward .
hii @asoplata ,can we move forward in this issue now ,as the currentscape needs to be on hold.
Hey @dikshant182004 , unfortunately this is not a good issue for new developers such as yourself for a number of reasons, including requiring extensive code changes and changing how we handle backwards-compatibility, in addition to many others.
Then I'll definitely want to give it a try.
hii @asoplata ,i was going through all the code and created a prototype of default.json in hierarchial format .I agreed upon the fact that it requires extensive code changes where the network configurations are being used . But after going through all the code ,I think we can complete this issue .plz have a look at this format
After going through the code base i think hnn_core/(param.py/network.py/network_model.py) require code changes ,if i am missing out some files in which these network configuration files are being used ,do tell me regarding that
@ntolley and @dylansdaniels , we should make a decision about the following: Do we want to:
-
Eventually, completely deprecate use of the "flat JSON" filetype (e.g. https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/param/default.json ) and the use of the
params.pymodule in favor of the GUI-compatible, larger "hierarchical JSON" (e.g. https://github.com/jonescompneurolab/hnn-core/blob/master/hnn_core/param/jones2009_base.json ) and thehnn_io.pymodule?- This would allow us to keep
params.py's behavior as-is (bugs/etc. included). - We would need to guide lab members and HNN users on converting "old workflows" that use any "flat JSON" files over to using the API to set parameters, etc. We should be doing this anyways, and still need to update and add tutorials that use
read_network_configurationandwrite...and that document the OVERALL workflow on a file-level basis. - All API code that uses the
params.pymodule (especiallynetwork_models.py, such asjones_model_2009()'sadd_drives_from_paramsoption) should be modified or replaced as-needed (for example, drive creation injones_model_2009(add_drives_from_params=True)using a new subfunction innetwork_models.pydirectly. - Perhaps the current
legacy_modecould be renamed tolegacy_mode_v1and code using the the "flat JSON" files separated into behavior called by a newlegacy_mode_v2flag. Again, to be clear, I'm not talking about deleting theparams.pymodule, but simply deprecating it while keeping it around. - This would take some work, BUT this could surface any existing bugs or surprising-behavior (such as found in #1180).
- This is what I prefer.
- This would allow us to keep
-
Alternatively, keep maintaining (as in keep-updating) both the older
params.pystyle and newerhnn_io.pystyle?- This is more maintenance burden in the long-term, since we would have to actively support both.
- We would still need to make a decision about which to include in tutorial content.
- This would involve making CHANGES to
params.pythat would probably NOT be backwards compatible with every use of the "flat JSON" approach, such as in #1180. It would be a bad and messy idea, but would introduce multiple "legacy_modes" toparams.pyalone.
-
Something else? For example, we could update
params.pyfor the not-backwards-compatible changes in #1180, but also communicate that we will be officially deprecating it at a certain time in the future (a year? two?). We could then say we will help anyone currently using HNN to port their workflow over (which we should do in the case of Option 1 anyways).
This decision impacts #1180 and #1140 as well.
Being able to provide variants on "how jones_2009_model() works now" versus a version with bug fixes would also allow us to address #996 and #997.