Skip to content
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

Removal of python-future compatibility #6377

Merged
merged 16 commits into from
May 7, 2024
Merged

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Jan 22, 2024

See also ome/omero-py#390

This PR reviews and cleans up all the code ensuring Python 2/3 compatibility including:

  • all imports of future and past modules
  • all checks of sys.version
  • all redundant imports of builtins

@jburel
Copy link
Member

jburel commented Mar 15, 2024

builtins still present in clitest/test_tag.py
Also in OmeroFS/test/drivers.py

@sbesson
Copy link
Member Author

sbesson commented Mar 15, 2024

For OMERO.fs, I don't think it's an import

filesets = eval(data, {"__builtins__": None}, {})

For the CLI tag test, I left the import as the input method is monkeypatched as part of the test

monkeypatch.setattr(builtins, "input", mock_input)

@jburel
Copy link
Member

jburel commented Mar 15, 2024

The changes look good
One thing I noticed is that we do not test various Python version. This is only tested with Python 3.10
We will need to modify the action https://github.com/ome/action-ice/blob/main/action.yml in order to support other Python version

@sbesson
Copy link
Member Author

sbesson commented Mar 15, 2024

Which use case are we trying to solve here? To the best of my knowledge of the GitHub workflows are executing the tests so I am not even sure the Python Ice bindings are a requirements for building.

@jburel
Copy link
Member

jburel commented Mar 15, 2024

It is when you build the gateway.
Anyway I am doing some tests with various Python versions. I will push a commit

@jburel
Copy link
Member

jburel commented Mar 16, 2024

Building with 3.12 leads to

  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/setuptools/_vendor/packaging/version.py", line 198, in __init__
    raise InvalidVersion(f"Invalid version: '{version}'")
setuptools.extern.packaging.version.InvalidVersion: Invalid version: 'UNKNOWN-ice36'

see https://github.com/jburel/openmicroscopy/actions/runs/8302622326/job/22725266554

@sbesson
Copy link
Member Author

sbesson commented Mar 16, 2024

Thanks @jburel, the UNKNOWN is likely related to the tag not being pulled. Independently of this, I have certainly seen similar errors recently with modern Python versions. I think one of the issues is that setuptools 69.0.0 now enforces strict compliance with PEP440 and we know the omero_version set via the build system and consumed in the various components/tools Python components inhttps://github.com/ome/openmicroscopy/blob/8398c91cfbb1f0e7a0ac1be24898132799639e3c/components/tools/python.xml#L15-L19 is not compliant.

components/tools probably needs to be reviewed and simplified (or extracted). Immediately two possible workarounds are:

  • pin setuptools<69 when install the requirements
  • set export NOPYTHON=1 i.e. skip the building of the Python components

@jburel
Copy link
Member

jburel commented Mar 17, 2024

@sbesson I have pushed commits to your PR so we run the build with Python 3.10, 3.11 an 3.12

@jburel jburel merged commit c105c06 into ome:develop May 7, 2024
4 checks passed
@sbesson sbesson deleted the future_cleanup branch May 7, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

2 participants