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.

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

XML read fixes in Plot classes #2623

Merged
merged 2 commits into from
Jul 25, 2023
Merged

Conversation

paulromano
Copy link
Contributor

Description

I noticed recently while generating a voxel plot that when I tried to read back a model with Model.from_xml that included a voxel plot, it raised an exception because it was expecting the "basis" to be set (which isn't written for a voxel plot). This PR fixes that and several other small bugs I noticed in XML read/write for Plots.

Cue @gridley telling us how much this manual XML reading/writing sucks!

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@gridley
Copy link
Contributor

gridley commented Jul 25, 2023

Nice! Funnily enough, it's probably my fault this broke after refactoring this a little for the ol' projection plots PR.

Copy link
Contributor

@gridley gridley left a comment

Choose a reason for hiding this comment

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

Nice fix on that orthographic width option, too! 😅

@gridley gridley merged commit 3152a2e into openmc-dev:develop Jul 25, 2023
16 checks passed
@paulromano paulromano deleted the plot-xml-fix branch July 25, 2023 14:21
stchaker pushed a commit to stchaker/openmc that referenced this pull request Oct 25, 2023
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