-
Notifications
You must be signed in to change notification settings - Fork 3
Fix for misc.save_script_parameters (Issue #136)
#140
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
Changes from 35 commits
8819fac
72fd4ed
cb78c24
921e31c
fd14aa5
535842a
28ab474
bd80bfc
87d9da0
ddae083
aa976d9
f429ede
f5355de
eb0a6c4
f96872b
2caa79f
dea08b4
df473bd
4e12f92
da163d3
c11c7d3
9ae8d76
c40da1c
0ca9092
cb793b4
b08af92
29128e1
9fafb73
ee9d0f0
b4be57b
43f3a1e
13c4a53
331777c
4ebc43f
9aa40c4
e75ff41
9d05d8a
27ad201
c23be08
c58fa13
e8140ed
0ea5eb6
0401201
8b208cb
24a8351
a4cb525
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| "bdvp", | ||
| "bigstitcher", | ||
| "biop", | ||
| "caplog", | ||
| "clij", | ||
| "Dscijava", | ||
| "flatfield", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Testing script for misc.save_script_parameters() | ||
|
|
||
| ```python | ||
| # @ String(label="Password", description="please enter your password", style="password") PASSWORD | ||
| # @ String(label="USERNAME", description="please enter your USR") USERNAME | ||
| # @ File(label="Path for storage", style="directory") outputPath | ||
| # @ Integer threshold | ||
| # @ Boolean taDa | ||
| # @ RoiManager rm | ||
| # @ CommandService command | ||
| # @ LogService sjlog | ||
|
|
||
| import os | ||
| from imcflibs.imagej import misc, omerotools | ||
|
|
||
| misc.save_script_parameters(outputPath, script_globals=globals()) | ||
| print("Saved params") | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,134 @@ | ||
| """Tests for `imcflibs.imagej.misc` utility functions.""" | ||
|
|
||
| import logging | ||
|
|
||
| import imcflibs.imagej.misc | ||
|
|
||
| from imcflibs.imagej.misc import bytes_to_human_readable | ||
| from imcflibs.imagej.misc import save_script_parameters | ||
|
|
||
|
|
||
| PASSWORD_ITEMS = ["OMERO_PASSWD"] | ||
|
|
||
|
|
||
| class ModuleItem: | ||
| """Mock for the org.scijava.module.ModuleItem interface.""" | ||
|
|
||
| def __init__(self, input_name): | ||
| """ModuleItem constructor. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| input_name : str | ||
| FIXME: describe (see the ScriptModule class) | ||
| """ | ||
| self.input_name = input_name | ||
|
|
||
| def getName(self): | ||
| """Getter method for the "input_name" attribute. | ||
|
|
||
| Returns | ||
| ------- | ||
| str | ||
| """ | ||
| return self.input_name | ||
|
|
||
|
|
||
| class ScriptInfo: | ||
| """Mock for the org.scijava.script.ScriptInfo class.""" | ||
|
|
||
| def __init__(self, input_names): | ||
| """ScriptInfo constructor. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| input_names : list(str) | ||
| FIXME: describe (see the ScriptModule class) | ||
| """ | ||
| self.input_names = [ModuleItem(x) for x in input_names] | ||
|
|
||
| def inputs(self): | ||
| """Get the list of input-objects. | ||
|
|
||
| Returns | ||
| ------- | ||
| list(ModuleItem) | ||
| """ | ||
| return self.input_names | ||
|
|
||
|
|
||
| class ScriptModule: | ||
| """Mock for the org.scijava.script.ScriptModule class.""" | ||
|
|
||
| def __init__(self, input_names, inputs): | ||
| """ScriptModule constructor. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| input_names : list(str) | ||
| The list of input names. FIXME: explain better. | ||
| inputs : dict | ||
| A dict having the `input_names` as keys. Values are representing the | ||
| content of the respective script parameter. | ||
| """ | ||
| self.info = ScriptInfo(input_names) | ||
| self.inputs = inputs | ||
|
|
||
| def getInfo(self): | ||
| """Getter method for the "info" attribute. | ||
|
|
||
| Returns | ||
| ------- | ||
| ScriptInfo | ||
| """ | ||
| return self.info | ||
|
|
||
| def getInputs(self): | ||
| """Getter method for the "inputs" attribute. | ||
|
|
||
| Returns | ||
| ------- | ||
| dict | ||
| """ | ||
| return self.inputs | ||
|
|
||
|
|
||
| def test_save_script_parameters_fail(caplog): | ||
| """Tests save_script_parameters with an invalid script_globals object.""" | ||
| caplog.clear() | ||
|
|
||
| save_script_parameters(script_globals=None, destination="") | ||
| assert "ScriptModule inspection failed" in caplog.messages[0] | ||
|
|
||
|
|
||
| def test_save_script_parameters(tmp_path, monkeypatch, caplog): | ||
| """Tests save_script_parameters.""" | ||
| caplog.set_level(logging.DEBUG) | ||
| caplog.clear() | ||
|
|
||
| base = tmp_path / "saved_parameters" | ||
| base.mkdir() | ||
|
|
||
| def _is_password_style(item): | ||
| return item.getName() in PASSWORD_ITEMS | ||
|
|
||
| monkeypatch.setattr(imcflibs.imagej.misc, "_is_password_style", _is_password_style) | ||
|
|
||
| script_module = ScriptModule( | ||
|
Member
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. @rohangirishrao let's discuss together what makes sense in terms of testing once you're back! 🚧 We also need to verify if the modified code in
Contributor
Author
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. TODO: it is better to keep them split, as using the same
Member
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.
Member
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.
Done (added a comment to explain the intent) in 8b208cb ✅ |
||
| input_names=["AAA", "BBB", "OMERO_PASSWD", "SJLOG", "NOT_THERE"], | ||
| inputs={"AAA": "aaa", "BBB": "bbb", "OMERO_PASSWD": "ultra-secret"}, | ||
| ) | ||
| script_globals = {"org.scijava.script.ScriptModule": script_module} | ||
| save_script_parameters(script_globals, destination=base) | ||
| assert "Skipping parameter from skip-list" in caplog.text | ||
| assert "Skipping password-style parameter" in caplog.text | ||
| assert "Unable to fetch value for parameter: NOT_THERE" in caplog.text | ||
| assert "Saved 2 parameters (skipped 1 password-style and 1 others)." in caplog.text | ||
| assert "Saved 2 script parameters to" in caplog.text | ||
|
|
||
| with open(str(base) + "/script_parameters.txt", "r") as f: | ||
| contents = f.read() | ||
| assert contents == "AAA: aaa\nBBB: bbb\n" | ||
|
|
||
|
|
||
| def test_bytes_to_human_readable_simple(): | ||
|
|
||
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 needs to be validated (cf. comment in tests)! 🚧
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.
The code in the state as shown here would ignore / exclude boolean parameters with a value of
False- as mentioned in the previous comment, this is now fixed in e8140ed.