Skip to content

First attempt to generalize unique metadata group interactions - #55

Merged
pshriwise merged 8 commits into
svalinn:mainfrom
gonuke:group_refactor
Oct 20, 2025
Merged

First attempt to generalize unique metadata group interactions#55
pshriwise merged 8 commits into
svalinn:mainfrom
gonuke:group_refactor

Conversation

@gonuke

@gonuke gonuke commented Oct 20, 2025

Copy link
Copy Markdown
Member

Fixes #54

Moves the code that handles interactions for all groups that may exist only once and use a key to define them:

  • get the group with that key
  • get the name of the group with that key
  • set the group with that key

It has been used for boundary data on Surfaces and material data on Volumes here.

@codecov

codecov Bot commented Oct 20, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.08%. Comparing base (0775624) to head (b382289).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
- Coverage   97.09%   97.08%   -0.02%     
==========================================
  Files           2        2              
  Lines         448      446       -2     
  Branches       52       48       -4     
==========================================
- Hits          435      433       -2     
- Misses          9       10       +1     
+ Partials        4        3       -1     
Flag Coverage Δ
unittests 97.08% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/pydagmc/dagnav.py Outdated
@pshriwise

Copy link
Copy Markdown
Member

If _boundary_prefix and _material_prefix use the same name (e.g. _metadata_prefix) we might be able to streamline the code further by removing some argument passing, but maybe at a loss of clarity. Thoughts @gonuke?

@gonuke

gonuke commented Oct 20, 2025

Copy link
Copy Markdown
Member Author

If _boundary_prefix and _material_prefix use the same name (e.g. _metadata_prefix) we might be able to streamline the code further by removing some argument passing, but maybe at a loss of clarity. Thoughts @gonuke?

I want to leave open the possibility of other metadata being stored on these groups such that there may be additional prefixes.

@pshriwise

Copy link
Copy Markdown
Member

If _boundary_prefix and _material_prefix use the same name (e.g. _metadata_prefix) we might be able to streamline the code further by removing some argument passing, but maybe at a loss of clarity. Thoughts @gonuke?

I want to leave open the possibility of other metadata being stored on these groups such that there may be additional prefixes.

Good call. I think I know what you have in mind.

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @gonuke!

I'm going to bypass the coverage check and merge. This improvement is worth the decrease of 0.02% in our 97% coverage I think 😃

@pshriwise
pshriwise merged commit 418e576 into svalinn:main Oct 20, 2025
2 of 3 checks passed
@gonuke

gonuke commented Oct 21, 2025

Copy link
Copy Markdown
Member Author

Thanks @gonuke!

I'm going to bypass the coverage check and merge. This improvement is worth the decrease of 0.02% in our 97% coverage I think 😃

Thanks!

I'm actually a little confused by the coverage test. There appear to be multiple metrics and I added tests that addressed the first metric. I can't for the life of me figure out where coverage is now reduced by this....

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.

Refactor interactions with metadata groups that can only exist once

2 participants