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

Start moving settingsValidation to the settings area #1895

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

john-science
Copy link
Member

@john-science john-science commented Sep 18, 2024

What is the change?

I have moved armi/operator/settingsValidation.py to armi/settings/settingsValidation.py, where it obviously belongs.

Why is the change being made?

Unfortunately, this would be a breaking change for like 6 downstream projects that I can see. So, for this first pass, I am not breaking the API, but adding in some backwards compatibility-saving tools.

I will try to update those downstream projects over the next few months, so we can make this change permanent.

But, for now, we need the backwards compatibility-saving temp module.

This is a first step in: #1880


Checklist

  • The release notes have been updated if necessary.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in pyproject.toml.

@john-science john-science added the cleanup Code/comment cleanup: Low Priority label Sep 18, 2024
Copy link
Contributor

@drewj-tp drewj-tp left a comment

Choose a reason for hiding this comment

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

I've confirmed the only diff between a git mv armi/operators/settingsValidation.py armi/settings is the lack of Query.__nonzero__

$ git mv .\armi\operators\settingsValidation.py .\armi\settings\
$ git diff --staged origin/settingsValidation_deprecate_move --  .\armi\settings\settingsValidation.py
diff --git a/armi/settings/settingsValidation.py b/armi/settings/settingsValidation.py
index ed11f5b3..324f2300 100644
--- a/armi/settings/settingsValidation.py
+++ b/armi/settings/settingsValidation.py
@@ -101,6 +101,8 @@ class Query:
             )
             raise

+    __nonzero__ = __bool__  # Python2 compatibility
+

Which is very okay.

Doc nit to render properly in the deprecated file and then we're good.

armi/operators/settingsValidation.py Outdated Show resolved Hide resolved
@john-science john-science merged commit 59c392c into main Sep 24, 2024
19 checks passed
@john-science john-science deleted the settingsValidation_deprecate_move branch September 24, 2024 20:56
drewj-tp added a commit that referenced this pull request Sep 26, 2024
…rams/1860

* origin/main:
  Adding ParameterCollection.where for conditional parameter iteration (#1899)
  Removing non-existent settings from test files (#1906)
  Removing reference to deprecated internal files (#1897)
  Fixing a problem with the latest release of h5py (#1907)
  Providing plugin hook getAxialExpansionChanger (#1870)
  Fixing typo in new settingsValidation imports (#1905)
  Startinf moving settingsValidation to the settings area (#1895)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code/comment cleanup: Low Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants