Open dagmc model file within a context manager #3706
Merged
+12
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GuySten
approved these changes
Jan 5, 2026
GuySten
changed the title
pshriwise
previously requested changes
Jan 5, 2026
openmc/dagmc.py
Outdated
| with h5py.File(self.filename) as dagmc_file_contents: | ||
| material_tags_hex = dagmc_file_contents['/tstt/tags/NAME'].get('values') | ||
| material_tags_ascii = [] | ||
| for tag in material_tags_hex: |
Contributor
There was a problem hiding this comment.
We access the HDF5 group here outside of the context manager, causing failures in testing.
I recommend we construct a list of the material strings inside the context manager block and iterate over the resulting list of strings in the loop here to do some cleanup.
Even better might be to create an internal function that sanitizes the material tag values and call that on each value of material_tags_hex.
Contributor
|
Looks like my comment came in a little behind your latest change @GuySten :) |
GuySten
dismissed
pshriwise’s stale review
This review's feedback has been fullfilled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
when getting the material names from a DAGMC file it appears that we don't close the file afterwards.
This PR change the code to use a context manager which automatically closes the file for us.
Fixes # (issue)
Checklist