-
Notifications
You must be signed in to change notification settings - Fork 8
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
Test nxdl hdf5 validator #280
base: fairmat
Are you sure you want to change the base?
Conversation
@sanbrock, @mkuehbach, and @lukaspie, In this PR I skipped doc generation functions on some weird nxdl and these nxdl are supposed for validation they might contain very weird definition and not meaningful for the outer world not even among us unless we do not know the purpose of the nxdl files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small questions. Should also run black to fix the CI/CD issues.
|
||
files = sorted(glob(str(app_def_path_glob))) | ||
for nexus_file in sorted(glob(str(contrib_def_path_glob))): | ||
delte_files = sorted(glob(str(contrib_def_path_glob))) + sorted( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not used at all
@@ -363,7 +369,12 @@ def find_definition_file(bc_name): | |||
Note that it first checks in contributed and goes beyond only if no contributed found | |||
""" | |||
bc_filename = None | |||
for nxdl_folder in ["contributed_definitions", "base_classes", "applications"]: | |||
for nxdl_folder in [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a bit awkward to also search the "dev_tools/tests/data"
folder here outside of the tests. Is it somehow possible to add this only in the test stage?
def try_find_default( | ||
logger, orig_elem, elem, nxdl_path, doc, attr | ||
): # pylint: disable=too-many-arguments | ||
def try_find_default(logger, orig_elem, elem, nxdl_path, doc, attr): # pylint: disable=too-many-arguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what changed here?
type="group" | ||
category="application" | ||
> | ||
<doc>This is a dummy NXDL to test out the dataconverter.</doc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not mention the dataconverter, which is a pynxtools concept. Just say it is a dummy for testing validation.
type="group" | ||
category="application" | ||
> | ||
<doc>This is a dummy NXDL to test out the dataconverter.</doc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
@@ -7,6 +7,11 @@ | |||
from ..globals.directories import get_xsd_file | |||
from ..nxdl import iter_definitions | |||
|
|||
SKIP_FILES_FROM_TESTS = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get why you skipped these files here, but should the doc generation not just work on them if they are valid files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my mistake, I just skip the doc generation not all the tests, where some tests validate the nexus data structure.
The files are not presentable to the outer users. I think Doc generation would make this NXDL accessible to the public. As the files are reference app defs for nexus file validator, later I will add further nxdl to make the validator fail.
And the doc generating does not give validity to the files. NXDL validation performed test in dev_tools
and validation
function while writing a nexus file from the NeXus template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The files are not presentable to the outer users. I think Doc generation would make this NXDL accessible to the public." I don't understand this, but this is probably related to my comment above. The user should never be able to use an NXDL in dev_tools/tests/data
, independent of the doc generation.
nxdl file path
along with contributed_definition, application and base_classes