8000 accept open enumerations as no real enumerations by sanbrock · Pull Request #558 · FAIRmat-NFDI/pynxtools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

accept open enumerations as no real enumerations #558

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

Merged
merged 3 commits into from
Feb 21, 2025

Conversation

sanbrock
Copy link
Collaborator

No description provided.

@sanbrock sanbrock requested a review from mkuehbach February 21, 2025 10:36
"""
Get the enumeration field from xml node
"""
enumeration = xml_node.find("nx:enumeration", __XML_NAMESPACES)
if enumeration is None:
return None
return None, None
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is not bool why not false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is neither True, nor False.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clear your return type annotation for the second one is bool though i.e. constrained would using that be cleaner,, agree does not change your logic in L550

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Collaborator
@mkuehbach mkuehbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments

Copy link
Collaborator
@mkuehbach mkuehbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sanbrock sanbrock merged commit 9a3c5ae into master Feb 21, 2025
17 checks passed
@sanbrock sanbrock deleted the open_enums_in_nomad branch February 21, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0