hnn-core icon indicating copy to clipboard operation
hnn-core copied to clipboard

MAINT: Upgrade ALL network configuration file formats (except for tests)

Open asoplata opened this issue 1 year ago • 13 comments

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.

asoplata avatar Jan 10 '25 17:01 asoplata

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

dikshant182004 avatar Jan 12 '25 09:01 dikshant182004

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

dikshant182004 avatar Jan 12 '25 09:01 dikshant182004

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.

asoplata avatar Jan 14 '25 16:01 asoplata

hii @asoplata thanks for guiding me i will start with the good first issue first then will move forward .

dikshant182004 avatar Jan 14 '25 16:01 dikshant182004

hii @asoplata ,can we move forward in this issue now ,as the currentscape needs to be on hold.

dikshant182004 avatar Feb 26 '25 12:02 dikshant182004

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.

asoplata avatar Feb 27 '25 16:02 asoplata

Then I'll definitely want to give it a try.

dikshant182004 avatar Feb 27 '25 17:02 dikshant182004

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

default2.json

dikshant182004 avatar Mar 01 '25 14:03 dikshant182004

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

dikshant182004 avatar Mar 01 '25 14:03 dikshant182004

@ntolley and @dylansdaniels , we should make a decision about the following: Do we want to:

  1. 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.py module 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 the hnn_io.py module?

    • 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_configuration and write... and that document the OVERALL workflow on a file-level basis.
    • All API code that uses the params.py module (especially network_models.py, such as jones_model_2009()'s add_drives_from_params option) should be modified or replaced as-needed (for example, drive creation in jones_model_2009(add_drives_from_params=True) using a new subfunction in network_models.py directly.
    • Perhaps the current legacy_mode could be renamed to legacy_mode_v1 and code using the the "flat JSON" files separated into behavior called by a new legacy_mode_v2 flag. Again, to be clear, I'm not talking about deleting the params.py module, 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.
  2. Alternatively, keep maintaining (as in keep-updating) both the older params.py style and newer hnn_io.py style?

    • 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.py that 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" to params.py alone.
  3. Something else? For example, we could update params.py for 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.

asoplata avatar Nov 04 '25 16:11 asoplata

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.

asoplata avatar Nov 06 '25 17:11 asoplata