version hardcodes
To dry
up run_test_old_and_new_code(),
it looks like we want a helper
that accepts a version number
such as "1.0.0" or "2.7.0".
In _dynamic_val() the 2.7.0 default
is very nice, but it didn't carry over
to this function.
And 2.7.0 appears in many logging statements
and calls.
At some point 2.7.1 will be relevant,
and we risk having a maintainer replace just a
subset of the versions that will need updating.
error handling
Some of these try blocks seem to correspond
to "impossible" or "never happens" conditions,
which your automated tests do not try to
exercise.
If so, then consider removing the try,
making the error fatal.
The default stack trace display will be
more informative than what the OP code displays.
If there's some specific logging requirement
in your specifications document,
then deal with it at top level in a uniform way,
instead of down within the various helpers.
There is no __init__() constructor,
indeed, no self.foo attributes at all.
This suggests that what you really
wanted was a collection of functions, rather than
methods hanging off an object instance.
It is usual to tack on a @staticmethod
decorator on methods which don't rely on self.
Here, we might have noticed that all the
helpers are static, and the pair of entry points
in the Public API only use self for dispatch
to a helper.
temp file cleanup
There's no """docstring""" on this helper
to explain the
concept of operations.
Consider having _dynamic_val() return a Path,
which caller is expected to .unlink(),
perhaps via a with context handler.
Currently it appears that such files
accumulate without bound.
wrong exit
if not os.path.exists(CONDA_PATH):
...
exit
The two development process difficulties here are:
- This doesn't work.
- You never ran that line of code.
The first is easy:
add import sys and call sys.exit()
The second is tougher, since it corresponds
to adopting a brand new habit and exercising
it each day that you write code.
I have personally written error handlers
like this, which error at exactly the time
you were hoping they would instead be helpful.
Recommend that you introduce a deliberate
typographic error ("C:/does/not/exist")
when defining that path, and verify it's
handled as you desire.
Then revert the typo, and don't worry about
that clause until the next time you mess
with the if test or the body.
A pretty common way to lose in such a handler
is with NameError.
# foo = 42
if is_failed():
print("failed, with foo =", foo)
Of course the code at one point
had properly defined foo.
But then as the months go on thing change,
and we never retried that print(),
so it's embarrassing when it finally triggers.
Routine static analysis with
ruff, pyright, or mypy
on each git push can help to surface
such a "whoops!".
lint
It is customary to use LOGGER.exception()
when reporting on some exception e,
in order to reveal the call site
via the displayed stack trace.
No need for an f-string
when there's no { } params,
such as f"foo = {foo}"
