Separate physics package and detector filters #1773
Merged
Conversation
This file contains 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
added 4 commits
January 8, 2025 08:56
Rather than using the update_physics_package function to also remove the package (set it to None), set it to None explicitly. The update function should only be used for actually updating the package. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
If the physics package is needed but missing, warn users and allow them to make the decision. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Users can have one without the other - remove any interdependencies that previously prevented this. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
|
Hello @bnmajor! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2025-01-10 01:16:50 UTC |
added 4 commits
January 9, 2025 11:55
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
The physics package option in the "edit" menu is no longer hidden based on instrument type. There are now two options: apply physics package, and edit physics package. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
- Whether or not the Physics Package is used is now indicated by a separate flag: "use_physics_package". - Users can only edit the physics package if "use_physics_package" is true - If "apply physics package" is selected the package manager dialog will automatically be displayed - While the physics package is no longer limited to PXRDIP and TARDIS it is still automatically applied after using the LLNL Import tool. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
bnmajor
force-pushed
the
physics-pkg-and-filters
branch
from
0098a81 to
9b6226e
Compare
bnmajor
force-pushed
the
physics-pkg-and-filters
branch
from
59a4f21 to
d8b11a4
Compare
added 5 commits
January 9, 2025 16:52
If "apply physics package" is selected make sure that we have a default physics package set if it was None. If we're setting the physics package to default we can infer we want to apply it as well. Keep the two states in sync. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
This is for backwards compatability. Previously, when detector coatings were set after using the LLNL import tool the "default" detector (used to represent the current detector during import) was not removed after import was complete. This means that some old state files will have this value and try to fetch this detector's information, even though it no longer exists. Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
Signed-off-by: Brianna Major <brianna.major@taloid.khq.kitware.com>
bnmajor
force-pushed
the
physics-pkg-and-filters
branch
from
d8b11a4 to
4a7a949
Compare
psavery
force-pushed
the
physics-pkg-and-filters
branch
from
db29ee3 to
1629fbb
Compare
psavery
reviewed
Jan 10, 2025
hexrdgui/main_window.py
Outdated
| dialog.show() | ||
| # Always assume Physics Package is needed for LLNL import | ||
| dialog.ui.complete.clicked.connect( | ||
| lambda: self.on_action_include_physics_package_toggled(True) |
There was a problem hiding this comment.
This would create a new connection every time the LLNL import tool is shown. I'll move this to setup_connections() instead. Otherwise, this could get triggered multiple times.
psavery
reviewed
Jan 10, 2025
hexrdgui/overlays/powder_overlay.py
Outdated
| # This will require a physics package | ||
| HexrdConfig().create_default_physics_package() | ||
| return |
There was a problem hiding this comment.
We need to continue creating a default physics package for backward compatibility with old state files, that had tth_distortion set but the physics package did not exist yet. I'll fix this.
I removed the `use_physics_package` variable from `HexrdConfig()` because it could have been simplified by simply checking if the physics package existed. I also fixed a few issues and cleaned up a few things. Signed-off-by: Patrick Avery <patrick.avery@kitware.com>
psavery
force-pushed
the
physics-pkg-and-filters
branch
from
1629fbb to
d363b6d
Compare
psavery
approved these changes
Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
All instruments should have the option to apply the physics package, the absorption correction, or both. This PR:
Relies on HEXRD/hexrd#741