-
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
New Attribute: speciesType #38
Conversation
Check if the optional `speciesType` attribute in the `SpeciesType` extension is a string and uses only allowed characters (e.g. no spaces).
@RemiLehe shall I push the updates to the example creator, writing the update: pushed. I skipped adding |
Add `speciesType` attributes to some records.
openpmd_validator/check_h5.py
Outdated
@@ -392,7 +397,7 @@ def check_root_attr(f, v): | |||
result_array += test_attr(f, v, "required", "iterationFormat", np.string_) | |||
|
|||
# optional but required for extensions | |||
result_array += test_attr(f, v, "optional", "openPMDextension", np.string_, "^[a-zA-Z0-9-;]+$") | |||
result_array += test_attr(f, v, "optional", "openPMDextension", np.string_, "^[a-zA-Z0-9\-;]+$") |
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.
note: this is a bug-fix for extension names from the last PR. The -
needs an escape, otherwise it's a regex range.
(\
is not allowed, would also need an escape.)
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.
Since regex can sometimes be cryptic, would you add a comment explaining what it does? Like
# Check that openPMDextension contains only letters, or digits, or the characters `-` and `;`
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.
yes, great point!
openpmd_validator/check_h5.py
Outdated
@@ -392,7 +397,7 @@ def check_root_attr(f, v): | |||
result_array += test_attr(f, v, "required", "iterationFormat", np.string_) | |||
|
|||
# optional but required for extensions | |||
result_array += test_attr(f, v, "optional", "openPMDextension", np.string_, "^[a-zA-Z0-9-;]+$") | |||
result_array += test_attr(f, v, "optional", "openPMDextension", np.string_, "^[a-zA-Z0-9\-;]+$") |
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.
Since regex can sometimes be cryptic, would you add a comment explaining what it does? Like
# Check that openPMDextension contains only letters, or digits, or the characters `-` and `;`
Check if the optional
speciesType
attribute in theSpeciesType
extension is a string and uses only allowed characters (e.g. no spaces).Introduced in openPMD 2.0.0: openPMD/openPMD-standard#180