Fix 189 by HannoSpreeuw · Pull Request #193 · transientskp/pyse

HannoSpreeuw · 2026-01-15T15:01:16Z

Closes #189

The command-line arguments that correspond to config.ImgConf and config.ExportSettings attributes, tap into the descriptions from the IMGCONF_HELP and EXPORTSETTINGS_HELP dicts. This design is DRY: no duplicate descriptions.
These two dicts are properly rendered by Sphinx.
All descriptions have been reviewed and some have been improved.
pyse- h now also shows the default values from ImgConf and ExportSettings.
Removal of redundant parentheses.
Reformatting from Black.
Grammar corrections in comments.
Reordering of code such that grid comes directly after back_size_x and back_size_y.
Addressed some linter warnings in cli.py.
A rebase was required after MR Show the pyse inputs, similar to -h #192

1) Move attribute descriptions of the "ImgConf" and "ExportSettings" dataclasses to two dicts. The advantage of this is that the descriptions that are applied in the cli, do not have to be duplicated. There seems to be no other way to make this DRY. Well, one could fill in "metadata" from the two dataclasses with these descriptions, but that would not be rendered by Sphinx. 2) Asserts have been added to establish that all attributes of "ImgConf" and "ExportSettings" have descriptions. 3) Fix linter warning "Type 'dict[str, Any]' doesn't have expected attribute 'strip'". 4) Reformatting from Black.

1) The descriptions of the command-line parameters that are also "ImgConf" or "ExportSettings" attributes, should come from the "IMGCONF_HELP" and "EXPORTSETTINGS_HELP" dicts that describe those attributes. In order to tap into those dicts the "imgconf_help" and "exportsettings_help" functions have been added to "cli.py". These functions add default values of the attributes to the text strings. 2) Removal of redundant parentheses. 3) Reformatting from Black. 4) Grammar corrections in comments. 5) Reordering of code such that "grid" comes directly after "back_size_x" and "back_size_y". # Conflicts: # sourcefinder/utility/cli.py

1) Render "IMGCONF_HELP" and "EXPORTSETTINGS_HELP" on a separate page: "config.rst". 2) Requires adding "config.rst" to "index.rst". 3) Also requires adding "docs.source.render_confs" to the Sphinx extensions listed in "conf.py". 4) The code to "unpack" and render "IMGCONF_HELP" and "EXPORTSETTINGS_HELP" is in "render_confs.py" and has been generated by ChatGPT 5.0.

Removed "args" following a linter hint.

This comment should make clear that the values in a config file will be overriden by command-line arguments but that they will override the defaults from "config.ImgConf" and "config.ExportSettings".

In a previous commit "type" and "value" were removed from "excepthook" following a linter directive: these arguments were not applied. This, however, led to a "mypy" error: " sourcefinder/utility/cli.py:495: error: Incompatible types in assignment (expression has type "Callable[[Any], Any]", variable has type "Callable[[type[BaseException], BaseException, TracebackType | None], Any]") [assignment] sys.excepthook = excepthook ^~~~~~~~~~ " The current commit comes from ChatGPT 5.0 and should satisfy both mypy and the linter.

We encountered a build error for the documentation: "ModuleNotFoundError: No module named 'docs'". Therefore, Sphinx cannot add the "docs.source.render_confs" extension. We moved "render_confs.py" to the new "sourcefinder/sphinx_ext" folder.

tmillenaar · 2026-01-16T15:51:57Z

sourcefinder/utility/cli.py

+        ) -> None:
+            """Enter post-mortem debugging on uncaught exceptions."""
+            if tb is not None:
+                pdb.post_mortem(tb)


Curious, what did you encounter that you needed this None check for?

Perhaps in a multithreading context an exception could be raised while the traceback was lost.

tmillenaar · 2026-01-16T15:58:36Z

sourcefinder/config.py

+    )
+
+
+test_imgconf_help_is_complete()


Very nice that we verify that these are the same. Such a setup is very likely to run out of sync if you forget to update so a regular check is very good to have. That said, are you sure you want to call this here? This runs any time you import the sourcefinder module, which already takes ~24 seconds for me to import. Is there a particular reason we run this check here rather then in the test suite?

Good point.
I will move it to the test suite.

tmillenaar · 2026-01-16T16:17:48Z

sourcefinder/config.py

+    field_names = _dataclass_field_names(ImgConf)
+    help_names = set(IMGCONF_HELP.keys())
+
+    missing = field_names - help_names


Since we do this check anyway it might also be nice to check if we ever remove a field_name but forgot to remove the help_name:

missing_help = field_names - help_names redundant_help = help_names - field_names if missing_help: raise KeyError("IMGCONF_HELP is missing descriptions for: " + ", ".join(sorted(missing_help))) if redundant_help: raise KeyError("IMGCONF_HELP has descriptions for non-existing variables: " + ", ".join(sorted(redundant_help)))

Nice. I will include that in the unit tests.

tmillenaar · 2026-01-16T16:17:56Z

sourcefinder/config.py

+    missing = field_names - help_names
+
+    assert (
+        not missing
+    ), "IMGCONF_HELP is missing descriptions for: " + ", ".join(
+        sorted(missing)
+    )


The check assert not missing is technically correct, but I read the inverse which threw me for a loop. Here you throw the error when the fields are "not missing".

We could do if missing: raise KeyError() or similar for readability, but since the current approach is technically correct I don't feel too strongly about it.

Right. Let me try to rephrase that.

tmillenaar · 2026-01-16T16:22:07Z

sourcefinder/config.py

+    )
+
+
+test_exportsettings_help_is_complete()


All the same applies here

Are we sure we want to run test on import?

assert not missing caused me some confusion at first

Could be nice to also do the inverse check: redundant_help = help_names - field_names

1) An assert statement in "utility/sourceparams.py" was issued at every "sourcefinder" import. This has now been moved to a new test module: "test_utility/test_sourceparams.py". 2) Besides asserting that all source parameters have a description, "test_sourceparams_descriptions_complete" also checks if all descriptions correspond to a source parameter. 3) Likewise for the completeness of "IMGCONF_HELP" and "EXPORTSETTINGS_HELP". 4) This means not only testing if every "ImgConf" and "ExportSettings" attribute has a description, but also checking if there are no excess descriptions. 5) "_source_params_descriptions" should not be private, therefore the underscore has been removed.

For Python 3.10 and 3.11 a "hatch run test.py3.11:pytest -vv test/test_utility/test_sourceparams.py" yielded " FAILED test/test_utility/test_sourceparams.py::test_sourceparams_descriptions_complete - TypeError: unsupported operand type(s) for 'in': 'str' and 'EnumType' " With these changes all four Python versions should pass.

HannoSpreeuw added 7 commits January 15, 2026 15:19

"type" and "value" are not applied in "excepthook"

195b463

"args" is not used by "handle_args"

76d9f9f

Removed "args" following a linter hint.

Make clear what the status of config.toml is

9e1987c

This comment should make clear that the values in a config file will be overriden by command-line arguments but that they will override the defaults from "config.ImgConf" and "config.ExportSettings".

HannoSpreeuw requested a review from tmillenaar January 15, 2026 15:01

HannoSpreeuw self-assigned this Jan 15, 2026

HannoSpreeuw added the docs label Jan 15, 2026

GH CI seems to be agnostic of "docs" folder.

257c6d2

We encountered a build error for the documentation: "ModuleNotFoundError: No module named 'docs'". Therefore, Sphinx cannot add the "docs.source.render_confs" extension. We moved "render_confs.py" to the new "sourcefinder/sphinx_ext" folder.

tmillenaar reviewed Jan 16, 2026

View reviewed changes

HannoSpreeuw added 3 commits January 20, 2026 19:31

No support for "Enum" classes needed here.

542848c

HannoSpreeuw merged commit c27418e into master Jan 20, 2026
9 checks passed

HannoSpreeuw deleted the Fix_189 branch January 20, 2026 19:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 189#193

Fix 189#193
HannoSpreeuw merged 11 commits intomasterfrom
Fix_189

HannoSpreeuw commented Jan 15, 2026

Uh oh!

tmillenaar Jan 16, 2026

Uh oh!

HannoSpreeuw Jan 16, 2026

Uh oh!

tmillenaar Jan 16, 2026

Uh oh!

HannoSpreeuw Jan 16, 2026

Uh oh!

tmillenaar Jan 16, 2026

Uh oh!

HannoSpreeuw Jan 16, 2026

Uh oh!

tmillenaar Jan 16, 2026

Uh oh!

HannoSpreeuw Jan 16, 2026

Uh oh!

tmillenaar Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		)


		test_imgconf_help_is_complete()

		)


		test_exportsettings_help_is_complete()

Conversation

HannoSpreeuw commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants