Skip to content

Add boundary property on Surface - #53

Merged
gonuke merged 5 commits into
svalinn:mainfrom
paulromano:boundary-property
Oct 20, 2025
Merged

Add boundary property on Surface#53
gonuke merged 5 commits into
svalinn:mainfrom
paulromano:boundary-property

Conversation

@paulromano

Copy link
Copy Markdown
Contributor

This PR adds the ability to set the boundary property on a surface. Not all codes are able to take advantage of this but it does look like DAGMC has code recognizing boundary:Reflecting and boundary:Vacuum properties.

@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.

Very small comment from me but otherwise looks good @paulromano!

Comment thread src/pydagmc/dagnav.py Outdated
Co-authored-by: Patrick Shriwise <pshriwise@gmail.com>

@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.

🚀

@gonuke gonuke 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.

One small edit to a comment to make it clear how general the group method is. Otherwise, looks good to me.

I see a value in generalizing some of these group operations in the future, but won't request it here :)

Comment thread src/pydagmc/dagnav.py Outdated
pshriwise and others added 2 commits October 20, 2025 12:02
Co-authored-by: Paul Wilson <paul.wilson@wisc.edu>
@codecov

codecov Bot commented Oct 20, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1af3754). Learn more about missing BASE report.
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/pydagmc/dagnav.py 96.15% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #53   +/-   ##
=======================================
  Coverage        ?   97.09%           
=======================================
  Files           ?        2           
  Lines           ?      448           
  Branches        ?       52           
=======================================
  Hits            ?      435           
  Misses          ?        9           
  Partials        ?        4           
Flag Coverage Δ
unittests 97.09% <96.15%> (?)

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.

@pshriwise

Copy link
Copy Markdown
Member

@gonuke brought your docstring fix in. Ready to merge in my eyes. Punch it in when you're ready plz.

@gonuke gonuke 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.

LGTM - thanks @paulromano

@gonuke
gonuke merged commit ab4fba8 into svalinn:main Oct 20, 2025
3 checks passed
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.

3 participants