validating an hdmf file can no longer be done by executing the module

dsleiter commented 3 years ago

Description

The HDMF documentation shows the following process for validating a file: https://hdmf.readthedocs.io/en/stable/validation.html

In talking with @rly, this used to work prior to extracting HDFM out of PyNWB, and now only works through the pynwb.validate module: https://pynwb.readthedocs.io/en/stable/validation.html

Two options for resolving are to:

remove this from the documentation
re-add this functionality matching the documentation

@rly suggested that we implement the second.

Update

Option 1. has been implemented in #482

This issue has been kept open until option 2 (i.e., adding validation functionality back) has been implemented.

Steps to Reproduce

Attempt to validate a hdmf file using the command provided in the documentation:

$ python -m hdmf.validate -p namespace.yaml test.h5

Environment

Python Executable: Conda
Python Version: Python 3.7
Operating System: Linux
HDMF Version: 2.2.0

Checklist

[x] Have you ensured the feature or change was not already reported ?
[x] Have you included a brief and descriptive title?
[x] Have you included a clear description of the problem you are trying to solve?
[x] Have you included a minimal code snippet that reproduces the issue you are encountering?
[x] Have you checked our Contributing document?

dsleiter commented 3 years ago

I tagged this as a bug because it sounds like it was intended functionality that no longer works, but I don't know what the bug criteria is for this project, so this could very easily be an enhancement.

I'd be happy to submit a PR for this. @rly suggested it could be very similar to this: https://github.com/NeurodataWithoutBorders/pynwb/blob/dev/src/pynwb/validate.py

oruebel commented 3 years ago

1. remove this from the documentation

2. re-add this functionality matching the documentation

Depending on the timeline for the fix, I would suggest to do 1 as a quick "hot fix" (i.e, simply comment this out in the documentation) and then implement 2 (and add back the appropriate documentation) once ready. If the timeline for implementing this is only a few weeks, then simply doing 2 is fine, but if plan to have additional releases in-between, then I would suggest to do 1 first.

I tagged this as a bug

That is a reasonable label. In this particular case, it would also be fine to label it as an enhancement since this is functionality that by itself has not existed in HDMF, but I think either option is fine.

dsleiter commented 3 years ago

I made a PR to update the documentation in #482. Should we keep this open, or create a new enhancement issue for implementing the documented functionality and close this issue?

It seems like implementing this in the code might not be a high priority since the validation functionality is currently available in pynwb, do you guys agree?

oruebel commented 3 years ago

Should we keep this open, or create a new enhancement issue

I think its simplest to just keep this issue open for now. I changed the label from bug to enhancement and added a short update to the description of the issue above to clarify what the state of this issue is and what remains to be done. Thanks @dsleiter for identifying this issue and the quick fix!

It seems like implementing this in the code might not be a high priority

I don't think there are any pressing use cases that need this functionality in HDMF itself right now, but I'm sure it will become necessary at some point. It is good to at least have an issue that documents that this still needs to be done.

hdmf-dev / hdmf