Add new macro PYTHON_VERSION

[stweil]

It is used for special handling of several versions of Python,
here to disable ocrd_kraken with Python 3.10.

Signed-off-by: Stefan Weil [email protected]

[stweil]

This is an extract from PR #295 and can be reused in PR #289.

[stweil]

We could use PYTHON_VERSION also as a hack to disable submodules which require old Tensorflow versions for newer versions of Python which don't provide those versions. That's much faster than the (cleaner) solution to check whether the required Tensorflow versions are available (as I did it in #295).

[bertsky]

This is an extract from PR #295 and can be reused in PR #289.

#289 already came first with that recipe BTW.

[bertsky]

This defies several layers of module selection mechanisms that are already in place.

If the user chose to install the ocrd_kraken module, then it is wrong to silently ignore it.

The proper way to remove a module for a certain Python version by default would be to include it in DISABLED_MODULES.

[kba]

So

DISABLED_MODULES ?= cor-asv-fst opencv-python ocrd_ocropy
ifeq ($(PYTHON_VERSION),3.10)                            
# Python 3.10.x does not work with current kraken.       
DISABLED_MODULES += ocrd_kraken                          
endif                                                    

would still allow overriding with OCRD_MODULES but disables it by default for 3.10?

[bertsky]

Not quite. That would still override any explicit user (or Docker) setting for DISABLED_MODULES. But we only want to override the actual default:

ifeq ($(PYTHON_VERSION),3.10)
# Python 3.10.x does not work with current kraken.
DISABLED_MODULES ?= cor-asv-fst opencv-python ocrd_ocropy ocrd_kraken
else
DISABLED_MODULES ?= cor-asv-fst opencv-python ocrd_ocropy
endif

[stweil]

My patch disables ocrd_kraken unconditionally for Python 3.10 (which is necessary because that combination does not work). The proposed alternatives don't do that. Therefore I don't think they are equally good or even better. I think the alternative proposed by @kba is fine and would work, too, but the alternative suggested by @bertsky would not disable unconditionally.

Both alternatives have the disadvantage of separating the implementation details for ocrd_kraken in the Makefile. As far as I remember it was you, @bertsky, who wanted to keep such details together.

[stweil]

We could discuss how to handle submodules which are disabled automatically because they don't work. Instead of silently disabling them, there could be a report (for example at the end of the build process) about those disabled submodules.

[bertsky]

My patch disables ocrd_kraken unconditionally for Python 3.10

exactly.

(which is necessary because that combination does not work)

thus, let it fail (loudly)!

Therefore I don't think they are equally good or even better.

As argued above, they are better (you did not address the arguments).

We could discuss how to handle submodules which are disabled automatically because they don't work. Instead of silently disabling them, there could be a report (for example at the end of the build process) about those disabled submodules.

That would only complicate matters further, and goes against the user expectations of running make. If a module does not work in your environment (whatever the cause or kind), then the simplest and uniform answer is that module's build failing (while others can continue if running with -k). Also, the build is incremental, so as soon as the user fixed it (by disabling the module or installing by hand), one can continue.

[stweil]

That pattern no longer works as soon as I want to disable other submodules for other Python versions!

[bertsky]

That pattern no longer works as soon as I want to disable other submodules for other Python versions!

If and when that happens, make still offers plenty of other syntax to achieve the same thing.

[stweil]

Then I need your help. How would that look like?

[bertsky]

For example, something along the lines of:

DISABLED_MODULES ?= $(DEFAULT_DISABLED_MODULES)
DEFAULT_DISABLED_MODULES = cor-asv-fst opencv-python ocrd_ocropy
ifeq ($(PYTHON_VERSION),3.10)
# Python 3.10.x does not work with current kraken.
DISABLED_MODULES += ocrd_kraken
endif
ifeq (...)
DISABLED_MODULES += ...
endif

[stweil]

That would change the user interface, right? Instead of providing DISABLED_MODULES they would have to use DEFAULT_DISABLED_MODULES. Of course all documentation would require an update, too.

Would that be acceptable and better than my original solution?

[bertsky]

No, it would not. The user does not need to know anything about DEFAULT_DISABLED_MODULES. In fact, that is the whole point (to have a dynamic default, but allowing explicit user-defined opt-outs as already documented).

[stweil]

I now pushed a commit which updates the handling of DISABLED_MODULES as you suggested (with a small fix).