-
Notifications
You must be signed in to change notification settings - Fork 128
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
SANS TOML V2: Polarization Options Added to User Files #38940
Open
cailafinn
wants to merge
16
commits into
mantidproject:main
Choose a base branch
from
cailafinn:38524_sans_polarisation_user_file_parsing
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
SANS TOML V2: Polarization Options Added to User Files #38940
cailafinn
wants to merge
16
commits into
mantidproject:main
from
cailafinn:38524_sans_polarisation_user_file_parsing
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
Covers both the electric and magnetic field inputs. RE mantidproject#38524
The old txt format won't be able to edit the values like the toml parser, but it will return a blank state so that the behaviour of the two parsers is identical. RE mantidproject#38524
Given that the new things we're adding to the user file aren't compatible with older versions of mantid, we're bumping up to TOML V2 so that we can keep being backwards compatible. This is mainly to stop new user files containing this information from being loaded by older versions, making it clear that you need to upgrade to use these new features and toml files. RE mantidproject#38524
0063f34
to
4d62ded
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
ISIS Team: LSS
Issue and pull requests managed by the LSS subteam at ISIS
SANS
Issues and pull requests related to SANS
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 of work
Summary of work
Allows specification of the polarisation options used in the experiment to be provided to the reduction via the user file (a file written in SANS' custom TOML format, usually one per experiment).
Fixes #38524
Further detail of work
Created
StatePolarization
. An JSON-serialisable object that holds the polarisation information to be passed around as part of SANSState to algorithms and interfaces.Refactored the TOML parser into a base and sub-classes to support...
Implemented SANS TOML V2 parser and schema. Upgrading to V2 means that files with the new information won't be supported by previous versions of Mantid. This is intentional. All V1 files continue to be backwards compatible, all V2 files will warn that the version is unsupported, forcing a version of Mantid to be used that can support the new polarisation options.
Refactored some tests to remove duplication caused by the additional parser.
Note: If required, this could be 2 PRs. The first implementing the State object and the second implementing the parsing.
To test:
loqdemo
in the training course data (you'll need that too): MaskFile_polarization.toml.zipReviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeepers
will take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.