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

Removing reference to r.o #901

Merged
merged 5 commits into from
Sep 28, 2022
Merged

Removing reference to r.o #901

merged 5 commits into from
Sep 28, 2022

Conversation

john-science
Copy link
Member

Description

As @jakehader mentioned in #830 , we have a strange "reactor.operator" object in a small number of places. Even though the primary reference is "operator.reactor".

This PR solves about half of those cases. The second half will be harder to unwind.

I have also opened some PRs in downstream repos to help pre-empt the second half of ticket #830.

Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any bug fixes or new features.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

@john-science john-science added enhancement New feature or request cleanup Code/comment cleanup: Low Priority labels Sep 26, 2022
@john-science john-science linked an issue Sep 26, 2022 that may be closed by this pull request
Remove operator from reactor #830
Open
@john-science john-science requested review from albeanth and opotowsky and removed request for jakehader September 26, 2022 23:50
opotowsky
opotowsky requested changes Sep 28, 2022
View reviewed changes
Copy link
Member

@opotowsky opotowsky left a comment

Choose a reason for hiding this comment

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

This looks good to me. Teensy update needed to a docstring.

armi/reactor/converters/uniformMesh.py Outdated Show resolved Hide resolved
@opotowsky
Copy link
Member

Curious -- why is distortionVsTime getting removed?

@john-science
Copy link
Member Author

Curious -- why is distortionVsTime getting removed?

Well, as far as I can tell, it references an Interface that hasn't existed in years, so it hasn't been USED in years.

Nebbychadnezzar
Nebbychadnezzar suggested changes Sep 28, 2022
View reviewed changes
armi/utils/reportPlotting.py Outdated Show resolved Hide resolved
@john-science john-science removed the request for review from albeanth September 28, 2022 22:55
@john-science john-science merged commit 646f23d into main Sep 28, 2022
@john-science john-science deleted the reactor_operator branch September 28, 2022 22:56
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 2022
@john-science @scottyak
Removing reference to r.o (terrapower#901)
ef9b9ee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nebbychadnezzar Nebbychadnezzar Nebbychadnezzar requested changes

@opotowsky opotowsky opotowsky approved these changes

Assignees
No one assigned
Labels
cleanup Code/comment cleanup: Low Priority enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove operator from reactor
3 participants
@john-science @opotowsky @Nebbychadnezzar