-
Notifications
You must be signed in to change notification settings - Fork 11
load args into DO metadata #82
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2bd8b50
initial commit
yucongalicechen 7c165c7
save keys to exclude as a global variable
yucongalicechen d3adca2
Merge branch 'main' into metadata3
yucongalicechen 96ba060
update with updated wavelength function
yucongalicechen 9135819
minor correction on wavelength docstring
yucongalicechen 78fce6c
use deep copy to avoid modification to original args
yucongalicechen 523808c
wrap up preprocessing steps (set inputs, wavelengths, etc) as a publi…
yucongalicechen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
from diffpy.labpdfproc.labpdfprocapp import get_args | ||
from diffpy.labpdfproc.tools import ( | ||
known_sources, | ||
load_metadata, | ||
load_package_info, | ||
load_user_info, | ||
load_user_metadata, | ||
|
@@ -277,3 +278,57 @@ def test_load_package_info(mocker): | |
actual_args = get_args(cli_inputs) | ||
actual_args = load_package_info(actual_args) | ||
assert actual_args.package_info == {"diffpy.labpdfproc": "1.2.3", "diffpy.utils": "3.3.0"} | ||
|
||
|
||
def _setup(mocker, user_filesystem): | ||
cwd = Path(user_filesystem) | ||
home_dir = cwd / "home_dir" | ||
mocker.patch("pathlib.Path.home", lambda _: home_dir) | ||
os.chdir(cwd) | ||
mocker.patch( | ||
"importlib.metadata.version", | ||
side_effect=lambda package_name: "3.3.0" if package_name == "diffpy.utils" else "1.2.3", | ||
) | ||
|
||
|
||
def _preprocess_args(args): | ||
args = load_package_info(args) | ||
args = load_user_info(args) | ||
args = set_input_lists(args) | ||
args.output_directory = set_output_directory(args) | ||
args.wavelength = set_wavelength(args) | ||
args = load_user_metadata(args) | ||
return args | ||
|
||
|
||
def test_load_metadata(mocker, user_filesystem): | ||
_setup(mocker, user_filesystem) | ||
cli_inputs = [ | ||
"2.5", | ||
".", | ||
"--wavelength", | ||
"1.54", | ||
"--user-metadata", | ||
"key=value", | ||
"--username", | ||
"cli_username", | ||
"--email", | ||
"[email protected]", | ||
] | ||
actual_args = get_args(cli_inputs) | ||
actual_args = _preprocess_args(actual_args) | ||
for filepath in actual_args.input_paths: | ||
actual_metadata = load_metadata(actual_args, filepath) | ||
expected_metadata = { | ||
"mud": 2.5, | ||
"input_directory": str(filepath), | ||
"anode_type": "Cu", | ||
"wavelength": 1.54, | ||
"output_directory": str(Path.cwd().resolve()), | ||
"xtype": "tth", | ||
"key": "value", | ||
"username": "cli_username", | ||
"email": "[email protected]", | ||
"package_info": {"diffpy.labpdfproc": "1.2.3", "diffpy.utils": "3.3.0"}, | ||
} | ||
assert actual_metadata == expected_metadata |
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,3 +213,36 @@ def load_package_info(args): | |
metadata = get_package_info("diffpy.labpdfproc") | ||
setattr(args, "package_info", metadata["package_info"]) | ||
return args | ||
|
||
|
||
def _set_anode_type(args): | ||
if args.wavelength in WAVELENGTHS.values(): | ||
args.anode_type = next(key for key, value in WAVELENGTHS.items() if value == args.wavelength) | ||
else: | ||
delattr(args, "anode_type") | ||
return args | ||
|
||
|
||
def load_metadata(args, filepath): | ||
""" | ||
Load metadata from args, | ||
except for anode type if wavelength does not match, and do not load output_correction or force_overwrite | ||
|
||
Parameters | ||
---------- | ||
args argparse.Namespace | ||
the arguments from the parser | ||
|
||
Returns | ||
------- | ||
A dictionary with all arguments from the parser | ||
""" | ||
|
||
args = _set_anode_type(args) | ||
metadata = vars(args) | ||
exclude_keys = ["output_correction", "force_overwrite", "input", "input_paths"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. save these as a global variabl above. |
||
for key in exclude_keys: | ||
metadata.pop(key, None) | ||
metadata["input_directory"] = str(filepath) | ||
metadata["output_directory"] = str(metadata["output_directory"]) | ||
return metadata |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this may be doing too much. i.e., "magic"?
I think the ucs are
uc1
UC2
I don't thing there is a reverse UC where a numeric value stored and lpp saves the anode type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we only took care of wavelength in the current code. anode type will be Mo if user doesn't specify it. So an input of wavelength=1.54 will produce anode_type=Mo in the current code. Shall I modify this in the wavelength function, or should I add a new function that take care of anode type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you are saying here. I want to remove the magic: "input of wavelength=1.54 will produce anode_type=Mo"
We don't want to guess where the user got the 1.54 A data from, maybe from a synchrotron?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we might need a separate function (or I can modify the existing wavelength function?) to remove the magic because in the previous issues we only checked that args has the correct wavelength (i.e. we didn't remove the anode type for UC2, but anode type is loaded automatically into the args because it's one of the arguments).
I got your point on not doing the reverse case.