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.

Sign up for GitHub

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

Jump to bottom

Validation fixes #350

Merged
merged 9 commits into from
Jun 28, 2024
Merged

Validation fixes #350

merged 9 commits into from
Jun 28, 2024

Conversation

domna
Copy link
Collaborator

@domna domna commented Jun 18, 2024 •
edited
Loading

Fixes

  • An issue when entries are search by naming convention (i.e., ENTRY for NXentry). Previously, it ignored if the found group has an actual name. This lead to wrong groups being returned.
  • Attaching sub-groups correctly in their parent concepts, i.e., my_data(NXdata) is contains DATA(NXdata) if the latter is part of a super-concept of the current appdef
  • Validation support if a specialised concept is given for for a required variadic group
  • Pins numpy<2.0.0

@domna domna requested a review from RonHildebrandt June 18, 2024 14:05
@domna domna self-assigned this Jun 18, 2024
domna
domna commented Jun 18, 2024
View reviewed changes
Copy link
Collaborator Author

@domna domna left a comment

Choose a reason for hiding this comment

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

@RonHildebrandt: Can you check if the warning is gone with this fix?

@coveralls
Copy link

coveralls commented Jun 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9566547897

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 96.591%
Files with Coverage Reduction New Missed Lines %
tests/dataconverter/test_nexus_tree.py 1 87.23%
Totals Coverage Status
Change from base Build 9516791059: -0.3%
Covered Lines: 595
Relevant Lines: 616
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9566881828

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 96.591%
Files with Coverage Reduction New Missed Lines %
tests/dataconverter/test_nexus_tree.py 1 87.23%
Totals Coverage Status
Change from base Build 9516791059: -0.3%
Covered Lines: 595
Relevant Lines: 616
💛 - Coveralls

@domna domna force-pushed the fix-naming-convention-adding branch from 5744b26 to 9a32059 Compare June 18, 2024 14:28
@coveralls
Copy link

coveralls commented Jun 18, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9567027186

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 96.916%
Files with Coverage Reduction New Missed Lines %
tests/dataconverter/test_nexus_tree.py 1 87.23%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 19, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9582994016

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.916%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

@domna
Copy link
Collaborator Author

domna commented Jun 21, 2024

@RonHildebrandt Everything should be fixed now. If you like you can give it another test. But I checked with your ellipsometry example files.

@coveralls
Copy link

coveralls commented Jun 21, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9611686574

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.916%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9611686574

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.916%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

RubelMozumder
RubelMozumder requested changes Jun 21, 2024
View reviewed changes
Copy link
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

Some of the function does not have any doc. Having a doc (even without the explanation of the function parameters) would be very helpful for future modification and development. Other than that I do not see any other big issue with this PR.

src/pynxtools/dataconverter/nexus_tree.py Outdated Show resolved Hide resolved
tests/dataconverter/test_validation.py Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jun 28, 2024 •
edited by github-actions bot
Loading

Pull Request Test Coverage Report for Build 9714482524

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.916%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 28, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9714518298

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 96.916%
Files with Coverage Reduction New Missed Lines %
tests/dataconverter/test_nexus_tree.py 1 87.23%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9714518298

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 96.916%
Files with Coverage Reduction New Missed Lines %
tests/dataconverter/test_nexus_tree.py 1 87.23%
Totals Coverage Status
Change from base Build 9516791059: 0.0%
Covered Lines: 597
Relevant Lines: 616
💛 - Coveralls

RubelMozumder
RubelMozumder approved these changes Jun 28, 2024
View reviewed changes
Copy link
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

LGTM!

src/pynxtools/dataconverter/validation.py Show resolved Hide resolved
@domna domna merged commit 56d13c1 into master Jun 28, 2024
11 checks passed
@domna domna deleted the fix-naming-convention-adding branch June 28, 2024 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RubelMozumder RubelMozumder RubelMozumder approved these changes

@RonHildebrandt RonHildebrandt Awaiting requested review from RonHildebrandt

Assignees

@domna domna

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@domna @coveralls @RubelMozumder