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

Add support for Python 3.11 #385

Merged
merged 6 commits into from
Nov 14, 2023
Merged

Add support for Python 3.11 #385

merged 6 commits into from
Nov 14, 2023

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented Oct 25, 2023

Python 3.11 has been released over a year ago and is available across several operating systems. This PR includes a minimal set of changes allowing OMERO.py to be functinal with Python 3.11.

Summary of changes

The biggest impact of the Python 3.11 was found to be in the dependencies of OMERO.py rather than the codebase itself

  • e1d0fa4 adds the Python 3.11 manylinux wheel package to the Tox framework and adds the py311 environment to tox.ini and the GitHub workflow definition to allow the testing under Python 3.11
  • the vendored fork of path.py uses the "universal newline" opening mode open(<file>, 'U') deprecated in Python 3.3. and removed in Python 3.11 1 . Eventually, there is probably a larger concerted effort to remove the usage of this fork in favor of pathlib available from the standard library. Within the context of this work, this was assessed to be a larger body of work. Instead e1d0fa4 simply adjusts omero_ext/path.py to use r since "universal newline" mode is used by default since Python 3.
  • the mox3 dependency 2 used by the unit tests was an unofficial port of the Google mox framework to Python 3. This library is unmaintained, has seen no releases since 3+ years and its usage was found to be incompatible with some of the Python 3.11 changes. 614a2e2 deprecates its usage in testlib and 986aa84 adjusts several tests to use pytest-mock 3, a thin-wrapper of the mock library available under the standard Python library

Testing

With this PR included, all CI builds and tests should keep passing. In addition, tt should be possible to use OMERO.py as a client as well as deploying OMERO.server in a Python 3.11 virtual environment.

Impact & open questions

  • 614a2e2 directly impacts OMERO.py integration tests maintained under https://github.com/ome/openmicroscopy. If no objection, I can open a companion Pull Request with the corresponding adjustments to use pytest-mock. Unlike the GitHub worklow builds, this might require additional infrastructure work to install the new dependency in the Jenkins CI environment where the integration tests are executed
  • test/unit/clitest/test_admin.py is still making heavy internal use of mox3 to test the omero admin subcommands. These tests are currently skipped in the GitHub workflow as they require 1- a valid OMERO.server environment referred by OMERODIR, 2- Java installed. The re-activation of these tests has been captured in 4. In the context of this work, I see three options:
    1. maintain the statu quo and keep the unit tests untested for years (and might not even work anymore)
    2. migrate these tests to ome/openmicroscopy so that they are executed in a valid environment
    3. keep these tests in omero-py possibly looking into a more extensive usage of Python mock library to remove the dependency on Java and maybe a valid server binary
      I'll hold off on any work on the above until we reach a consensus.

Footnotes

  1. https://docs.python.org/3/whatsnew/3.11.html#porting-to-python-3-11

  2. https://pypi.org/project/mox3/

  3. https://pytest-mock.readthedocs.io/en/latest/

  4. https://github.com/ome/omero-py/issues/61

@jburel
Copy link
Member

jburel commented Oct 27, 2023

614a2e2 directly impacts OMERO.py integration tests maintained under https://github.com/ome/openmicroscopy. If no objection, I can open a companion Pull Request with the corresponding adjustments to use pytest-mock. Unlike the GitHub worklow builds, this might require additional infrastructure work to install the new dependency in the Jenkins CI environment where the integration tests are executed

It will make sense to adjust the tests under https://github.com/ome/openmicroscopy. We have several repositories using mox3 e.g. omego, scc, cli-render. Is the idea to dispatch the work?

The OMERO-test-integration and OMERO-training jobs will need to be adjusted.

Regarding the test mentioned above test/unit/clitest/test_admin.py. The tests could probably be migrated under https://github.com/ome/openmicroscopy. Outside the scope of this PR

@jburel
Copy link
Member

jburel commented Oct 27, 2023

mox3 should also be removed from

"mox3", "IceGrid", "mx", "matplotlib",

@jburel
Copy link
Member

jburel commented Oct 27, 2023

tests_require=[ 'pytest', 'mox3', ],
in setup.py will need to be adjusted accordingly

@sbesson
Copy link
Member Author

sbesson commented Oct 27, 2023

It will make sense to adjust the tests under https://github.com/ome/openmicroscopy.

Opened as ome/openmicroscopy#6369. I can open associated devspace PRs if needed but eventually defer to you guys on when/how these changes should be integrated into the CI environment.

We have several repositories using mox3 e.g. omego, scc, cli-render. Is the idea to dispatch the work?

The primary intent was to have OMERO working on Python 3.11. The mox3 removal really comes as a requirement as soon as tests are executed against Python 3.11 but I can certainly look into auditing the OMERO plugin repositories as I expect the changes are minimal.
omego and scc are outside this scope of this work from my side.

@jburel
Copy link
Member

jburel commented Oct 27, 2023

Regarding devspace, i will update my working branch (rocky9 support) which is what we are moving towards

Use mock.patch and mock.patch.object to patch the CLI methods
associated with the admin subcommands
Use mock asserts to confirm the right arguments are being passed
to popen, call, err and out
@sbesson
Copy link
Member Author

sbesson commented Oct 30, 2023

tests_require=[ 'pytest', 'mox3', ], in setup.py will need to be adjusted accordingly

As shown by the builds of 8360c3f, an unfortunate side-effect is that the omero admin unit tests currently skipped by GitHub Actions and discussed in the description of this PR depended on mox3 for the MockCLI class.

d378c39 tackles the problem by rewriting these tests to use the mock package and get rid entirely of test/unit/clitest/mocks.py. These tests are passing in my local environment so hopefully they are in much better place to be migrated into the appropriate location as a follow-up of this work.

@jburel
Copy link
Member

jburel commented Nov 9, 2023

Note that b4a5342 is required to get the test green in omero-web with Python 3.11 cc @knabar

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@jburel
Copy link
Member

jburel commented Nov 14, 2023

Discussed today. Merging and tagging as 5.17.0

@jburel jburel merged commit cf37571 into ome:master Nov 14, 2023
8 checks passed
@sbesson sbesson deleted the python311 branch November 14, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants