Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion related to #542 #543

Open
t-b opened this issue Jul 4, 2022 · 4 comments
Open

Discussion related to #542 #543

t-b opened this issue Jul 4, 2022 · 4 comments
Labels

Comments

@t-b
Copy link
Collaborator

t-b commented Jul 4, 2022

@njmei
The original idea was that ipfx and MIES use the same nwb schema version.
As there is currently no support for multiple schema versions in one NWB file. I also doubt that this can be ever done properly.

From the requirements.txt of pynwb

pynwb>=1.3.2,<2.0.0

and dandi-cli

https://github.com/dandi/dandi-cli/blob/39dc4f55caec0c92cdfed24db1bc604f988fc0e7/setup.cfg

one can work with older pynwb versions as well.

MIES supports NWB schema version 2.2.0, see https://github.com/AllenInstitute/IPNWB/blob/main/index.rst.

So in essence I think using the corresponding old pynwb version in ipfx, something like 1.2.1, would reduce the fragility introduced with #542.

@t-b t-b added the question label Jul 4, 2022
@njmei
Copy link
Contributor

njmei commented Jul 4, 2022

@t-b Thanks for the comment! Since IPFX is intended to be used by end-users (at least based on my understanding), pinning the pynwb version to something old like 1.2.1 is probably not ideal since it will force end-users to create a separate python environment just for IPFX and will limit the ability for others to import IPFX modules/functionality into their own custom analyses...

@tmchartrand
Copy link
Collaborator

@t-b I also agree with @njmei that pinning to old versions isn't going to be a workable solution for IPFX. In fact, we're likely going to have some need to jump to using newer pynwb version at some point to use some metadata fields in newer versions of the schema.
This would be much less challenging if NWB/HDMF stuck to a policy of breaking changes on major versions only - this issue started because of a minor but breaking schema change that snuck through in hdmf (I think the field just didn't have a type specified before).
I think it would still be reasonable to assume that won't happen often, so we'll need to update MIES to match this schema change, but in general can probably let it lag behind and swap a file's cached schema for a newer version when IPFX modifies it. If we add in a validation call before saving it, then we can flag any issues early.

@t-b
Copy link
Collaborator Author

t-b commented Aug 1, 2022

Thanks for your replies.

Yes (obviously) it is not nice to have to stick to an old version of pynwb.

In fact, we're likely going to have some need to jump to using newer pynwb version at some point to use some metadata fields in newer versions of the schema.

If we do that, we should coordinate that between MIES and ipfx so that both rely on a next fixed good version for a while. I've created AllenInstitute/IPNWB#37 to track that.

If we add in a validation call before saving it, then we can flag any issues early.

Yes, that should definitly be done and help in finding issues.

@t-b t-b closed this as completed Aug 1, 2022
@tmchartrand
Copy link
Collaborator

@njmei just thinking maybe we should reopen this as a reminder of the need for further MIES/IPFX coordination, linked to Thomas's issue on the IPNWB end.

@t-b t-b reopened this Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants