Fix for ` misc.save_script_parameters` (Issue #136) by rohangirishrao · Pull Request #140 · imcf/python-imcflibs

rohangirishrao · 2026-03-25T14:20:32Z

The function now accepts globals() as a parameter - this is a workaround as calling globals() from the method did not work, see #136.

Attempt to fix the problem in a new branch, see imcf#136 for details.

This is necessary as calling globals from inside the method lead to no variables from the call time.

codecov · 2026-03-25T14:23:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32%. Comparing base (376ceb7) to head (a4cb525).
⚠️ Report is 47 commits behind head on devel.

Additional details and impacted files

@@         Coverage Diff          @@
##           devel   #140   +/-   ##
====================================
+ Coverage     30%    32%   +2%     
====================================
  Files         25     25           
  Lines       1748   1782   +34     
====================================
+ Hits         526    563   +37     
+ Misses      1222   1219    -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

rohangirishrao · 2026-03-27T09:33:36Z

Is there anything to be done for the codecov failing? There is no way to really test this part

The previous approach of "fetching" it within the function didn't work out, so having it as an optional parameter doesn't make much sense any more. 🚨 WARNING 🚨 This change will break any existing scripts that were using the function from a pre-release of imcflibs!

With switching to monkeypatch, it's not needed any more.

ehrenfeu · 2026-04-08T12:17:10Z

tests/test_misc.py

@@ -115,13 +115,15 @@ def _is_password_style(item):
    monkeypatch.setattr(imcflibs.imagej.misc, "_is_password_style", _is_password_style)

    script_module = ScriptModule(


@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 save_script_parameters() still works in ImageJ / Jython as expected / intended! 🚧

TODO: it is better to keep them split, as using the same output_dir from script params for example, in all methods is easier. The name of the file can be changed separately from the path, we have other methods where we pass the output_dir so for consistency, I think it's best to keep both.

_is_password_style() looks great, thanks for that! I think the tests are good, we could test for the save_file_name being correct, but maybe that's not so necessary. We can take a look together tomorrow, and I'll see if I can test it in Fiji till then.

Regarding the two separate parameters: understood, that's what my (very faint) memory told me anyway, hence the question. ➡️ ✅

The saved file is already re-read by the test, so that should be fine. Regarding testing in Fiji, I have now updated the corresponding file in tests/interactive-imagej/ (and renamed it from test_save_params.md to save-script-parameters.py), which also lead to the discovery of a bug that got fixed in e8140ed. Let's have another look together tomorrow, but I believe we should be fine here.

it is better to keep them split, as using the same output_dir from script params for example, in all methods is easier. The name of the file can be changed separately from the path, we have other methods where we pass the output_dir so for consistency, I think it's best to keep both.

Done (added a comment to explain the intent) in 8b208cb ✅

ehrenfeu · 2026-04-08T12:21:11Z

src/imcflibs/imagej/misc.py

+            # TODO: discuss if this approach is fine within Fiji/Jython
+            try:
+                val = inputs.get(key)
+                if not val:  # required for testing in CPython
+                    raise KeyError("failure looking up value for '%s'" % key)
+                f.write("%s: %s\n" % (key, str(val)))
+                saved += 1
+            except:
+                log.warning("Unable to fetch value for parameter: %s", key)
+                pass


🚧 This needs to be validated (cf. comment in tests)! 🚧

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.

The previous "if not val" also triggers if the parameter is a boolean that has the value "False" and therefore wrongly excludes those. Checking for "is None" only triggers if the .get() call fails and returns its default value (None).

The workflow described in TESTING.md states to drag and drop the script onto Fiji, so for the time being let's stick to that

ehrenfeu · 2026-04-10T07:30:57Z

Actually... While doing all of this a few more ideas for this function popped 💡 up:

It would be nice if the function were to automagically determine whether the parameters should be saved or not. Then it could simply always be called from our scripts and do its service. A reasonable 🤔 approach would be to have an empty default value for the target location by using destination="" and skip parameter saving if it's not defined.
Instead of supplying the saving location explicitly in parameter destination, we could modify it such that the location is defined within the Script Parameters (see illustration example below) using a default parameter name (e.g. OUTPUT_PATH or similar) and only have an optional parameter for save_script_parameters that allows to specify which Script Parameter contains the saving location if it differs from the standard.
To achieve this, processing ⚙️ of the Script Parameters in save_script_parameters would need to be modified such that it works through the items obtained via script_globals and assembles a list of things 📑 to be saved while determining the saving location 💾 at the same time - only using open(out_path, "w") as f: after all Script Parameters have been processed (as only then out_path would be known).
We should consider to construct the file name used to save the parameters and values automatically, e.g. following this pattern: <calling_script_name>_run-<timestamp>_params.txt - this would allow for saving the parameters at each run without having to worry (too much) about overwriting previously saved ones. For hints on how to identify the name of the calling script, this SO thread might contain useful ideas - 🚨 careful though, it's pretty old and might not work the same way in ImageJ-Jython! ☝🏼The .py suffix should be stripped from the pattern.
In case saving the parameters in the OUTPUT_PATH location suggested above fails (location might be read-only, ...), falling back to e.g. the user's home directory would be nice (plus issuing a ⚠️ warning message obviously).

`example1.py`

#@ File(label="Input data", style="directory") INPUT_PATH
#@ File(label="Path for results", style="directory") OUTPUT_PATH

from imcflibs.imagej.misc import save_script_parameters

save_script_parameters(script_globals=globals())

Running the above with OUTPUT_PATH=/data/project23/results would result in this:

Saved 2 script parameters to: /data/project23/results/example1_run-2026-04-10--10-41_params.txt

`example2.py`

This illustrates the option of using a different Script Parameter to define where the parameters should be saved:

#@ File(label="Input data", style="directory") INPUT_PATH
#@ File(label="Path for results", style="directory") OUTPUT_PATH
#@ File(label="Path to store parameters", style="directory") PARAMS_PATH

from imcflibs.imagej.misc import save_script_parameters

save_script_parameters(script_globals=globals(), location_param="PARAMS_PATH")

Running this with PARAMS_PATH=/data/project23/runs would give:

Saved 3 script parameters to: /data/project23/runs/example2_run-2026-04-10--10-50_params.txt

🤯 😳

Now, obviously - these are a lot of changes, and they definitely qualify for a separate issue. In my opinion, the main question (apart from discussing the stuff mentioned above) is whether to have this for the 2.0.0 milestone or if this should go into a subsequent release (🚨 having a high probability of becoming a breaking change and hence requiring a new major release...)?

ehrenfeu · 2026-04-10T12:10:59Z

After discussing today we decided to:

merge this PR into devel - it's about fixing the broken function, which is done ✅
move 🔜 the suggested changes to a new issue (or multiple ones) and then decide there 🔀 about the milestone question

ehrenfeu and others added 3 commits January 23, 2026 12:44

Re-add misc.save_script_parameters()

8819fac

Attempt to fix the problem in a new branch, see imcf#136 for details.

Run black

72fd4ed

Add parameter so that globals can be passed directly

cb78c24

This is necessary as calling globals from inside the method lead to no variables from the call time.

rohangirishrao changed the title ~~Fix for m (Issue #136)~~ Fix for misc.save_script_parameters (Issue #136) Mar 25, 2026

This was referenced Mar 25, 2026

Issue accessing globals() from library #136

Closed

Update changelog for 2.0.0 #126

Open

rohangirishrao added 2 commits March 25, 2026 18:06

Add testing script for save parameters method

921e31c

Remove false file, add testing script for parameter saving

fd14aa5

ehrenfeu self-requested a review March 25, 2026 21:27

ehrenfeu assigned rohangirishrao Mar 25, 2026

ehrenfeu added the enhancement New feature or request label Mar 25, 2026

ehrenfeu added this to IMCF-Fiji Mar 25, 2026

ehrenfeu added this to the 2.0.0 milestone Mar 25, 2026

ehrenfeu linked an issue Mar 25, 2026 that may be closed by this pull request

Issue accessing globals() from library #136

Closed

Merge remote-tracking branch 'imcf/devel' into save-script-params

535842a

ehrenfeu moved this to In review in IMCF-Fiji Mar 25, 2026

ehrenfeu added 12 commits April 7, 2026 17:45

Merge branch 'devel' into pr/140

28ab474

Place elementary explanation in first section

bd80bfc

Improve docstring examples section

ddae083

Simplify code by using try/except

aa976d9

Update mocks version in TESTING docs

f429ede

Attempt to make code testable

f5355de

Add pytest-mock devel dependency

eb0a6c4

Attempt tests for save_script_parameters

f96872b

Add todo

2caa79f

Add FIXME

dea08b4

Merge branch 'devel' into pr/140

df473bd

ehrenfeu added 12 commits April 8, 2026 09:14

Use Python2 compatible syntax

0ca9092

Extra safety net for pyproject.toml

cb793b4

Add test_save_script_parameters_fail

b08af92

Add counts and log messages

29128e1

Improve test by switching to monkeypatch

9fafb73

Remove pytest-mock devel dependency again

ee9d0f0

With switching to monkeypatch, it's not needed any more.

Ensure working code path on CPython

b4be57b

Add log message to except block

43f3a1e

Cover more code paths in test

13c4a53

Exclude function from coverage

331777c

Merge branch 'devel' into pr/140

4ebc43f

Revert to default coverage exclude pragma

9aa40c4

ehrenfeu reviewed Apr 8, 2026

View reviewed changes

ehrenfeu added 9 commits April 8, 2026 16:53

Whitespace / formatting

e75ff41

Explain excluded keys

9d05d8a

Update required version of imcf-fiji-mocks

27ad201

Use mocks from imcf-fiji-mocks

c23be08

Merge branch 'devel' into pr/140

c58fa13

Explicitly check for None

e8140ed

The previous "if not val" also triggers if the parameter is a boolean that has the value "False" and therefore wrongly excludes those. Checking for "is None" only triggers if the .get() call fails and returns its default value (None).

Update script to match new function signature

0ea5eb6

Align with testing instructions

0401201

The workflow described in TESTING.md states to drag and drop the script onto Fiji, so for the time being let's stick to that

Explain separate parameters

8b208cb

rohangirishrao and others added 2 commits April 10, 2026 11:27

Correct to backslash

24a8351

Fix syntax destroyed by ruff-formatter

a4cb525

ehrenfeu merged commit 0be22b3 into imcf:devel Apr 10, 2026
6 checks passed

github-project-automation bot moved this from In review to Done in IMCF-Fiji Apr 10, 2026

ehrenfeu mentioned this pull request Apr 10, 2026

Generalization of save_script_parameters #145

Open

rohangirishrao deleted the save-script-params branch April 10, 2026 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for `misc.save_script_parameters` (Issue #136)#140

Fix for `misc.save_script_parameters` (Issue #136)#140
ehrenfeu merged 46 commits intoimcf:develfrom
rohangirishrao:save-script-params

rohangirishrao commented Mar 25, 2026

Uh oh!

codecov bot commented Mar 25, 2026 •

edited

Loading

Uh oh!

rohangirishrao commented Mar 27, 2026

Uh oh!

ehrenfeu Apr 8, 2026

Uh oh!

rohangirishrao Apr 9, 2026

Uh oh!

ehrenfeu Apr 9, 2026

Uh oh!

ehrenfeu Apr 9, 2026

Uh oh!

ehrenfeu Apr 8, 2026

Uh oh!

ehrenfeu Apr 9, 2026

Uh oh!

ehrenfeu commented Apr 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

ehrenfeu commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -115,13 +115,15 @@ def _is_password_style(item):
		monkeypatch.setattr(imcflibs.imagej.misc, "_is_password_style", _is_password_style)

		script_module = ScriptModule(

Conversation

rohangirishrao commented Mar 25, 2026

Uh oh!

codecov bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rohangirishrao commented Mar 27, 2026

Uh oh!

ehrenfeu Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

rohangirishrao Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ehrenfeu Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ehrenfeu Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ehrenfeu Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

ehrenfeu Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

ehrenfeu commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

example1.py

example2.py

Uh oh!

Uh oh!

ehrenfeu commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Mar 25, 2026 •

edited

Loading

ehrenfeu commented Apr 10, 2026 •

edited

Loading

`example1.py`

`example2.py`