Removal of python-future compatibility code #531

knabar · 2024-02-06T21:47:36Z

This PR implements issue #528 to review the OMERO.web source code to update all paths with either Python 2 or Python 2/3 logic to pure Python 3+, similar to @sbesson's work for OMERO.py at ome/omero-py#390.

for more information, see https://pre-commit.ci

knabar · 2024-02-07T21:01:49Z

@sbesson I have gotten to the point where tests are now failing because the underlying omero-py is not Python 3.12 compatible, so we have to coordinate somehow

sbesson

From a code review perspective, the series of changes proposed here look good and I think we are ready to include this the nightly OME CI builds and schedule a formal round of functional testing.

For #531, one possible temporary approach to run the tests on Python 3.12 would be sbesson@2edac67. Once we have an omero-py release with Python 3.12 support, this commit will need to be reverted. I suspect we'll also want to update

omero-web/setup.py

Line 54 in 9841167

"omero-py>=5.7.0",

.

omeroweb/settings.py

tox.ini

sbesson

With the caveat that the temporary commit pointing to my branch should be updated before merging, all changes look sensible to me from an initial code review. I'll let the OME team schedule the functional testing of this set of maintenance/Python 3.12 support PR.

jburel · 2024-02-09T11:34:17Z

2edac67 broke today's installation of omero-server. It highlights an issue in the ci-build

sbesson · 2024-02-09T11:50:28Z

I think the issue is with 52c84a6 rather than the tox.ini change which is unused by OME CI as far as I know. Here's the failing workflow as I understand it:

OMERO-python-superbuild-build creates distributions of the omero-web and omero-py packages including all included PRs
OMERO-server tries to install these packages in order. When installing omero-web, the install_requires modifies 52c84a6 checks out my omero-py branch from GitHub and runs setup.py
unlike the artifacts from the Jenkins job above, the Git checkout does not include a target folder and omero-py tries to download blitz-python - see https://github.com/ome/omero-py/blob/1ca2f8729e684c5e702931ac4c7b2f768d950d0e/setup.py#L60-L64
the JENKINS_URL is misconfigured on https://merge-ci.openmicroscopy.org/jenkins causing the job to fail.

I suspect force pushing away 52c84a6 should be sufficient to keep testing this PR through GitHub actions (as per the tox.ini workflow) and meet the requirement of the OME Jenkins pipeline.

jburel · 2024-02-09T11:50:34Z

I have now removed the excluded label since I found the issue in the CI job leading to the unexpected behaviour

jburel · 2024-02-09T11:58:58Z

Sorry yes, wrong cut and paste.
Problem is sorted now
note that this is not due to the misconfiguration of the JENKINS_URL

No need to force push the commit

jburel · 2024-02-09T12:02:59Z

The JENKINS_URL has been reviewed in ome/devspace@1859719

jburel · 2024-02-09T12:12:13Z

See ome/devspace@096b024 for the adjustment (new CI)
Similar changes made to merge-ci https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-server/402/

snoopycrimecop · 2024-02-10T00:33:31Z

Conflicting PR. Removed from build OMERO-python-superbuild-push#2. See the console output for more details.
Possible conflicts:

PR Right panel performance fixes #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/views.py

--conflicts

snoopycrimecop · 2024-02-10T00:43:25Z

Conflicting PR. Removed from build OMERO-python-superbuild-push#452. See the console output for more details.
Possible conflicts:

PR Right panel performance fixes #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/views.py

--conflicts

snoopycrimecop · 2024-02-11T00:34:05Z

Conflicting PR. Removed from build OMERO-python-superbuild-push#3. See the console output for more details.
Possible conflicts:

PR Right panel performance fixes #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/views.py

--conflicts

snoopycrimecop · 2024-02-11T00:44:38Z

Conflicting PR. Removed from build OMERO-python-superbuild-push#453. See the console output for more details.
Possible conflicts:

PR Right panel performance fixes #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/views.py

--conflicts

snoopycrimecop · 2024-02-12T00:45:26Z

Conflicting PR. Removed from build OMERO-python-superbuild-push#454. See the console output for more details.
Possible conflicts:

PR Right panel performance fixes #17 will-moore 'Right panel performance fixes'
- omeroweb/webclient/views.py

~~--conflicts~~ Conflict resolved in build OMERO-python-superbuild-push#4. See the console output for more details.

jburel · 2024-02-27T21:44:07Z

What is the reason for dropping Python 3.8 from the testing matrix?

knabar · 2024-02-27T21:53:34Z

What is the reason for dropping Python 3.8 from the testing matrix?

Just an oversight, I made a mistake undoing the removal.

jburel · 2024-02-27T21:53:37Z

omeroweb/webgateway/marshal.py

@@ -296,8 +295,8 @@ def set_if(k, v, func=lambda a: a is not None):

    def decode(shape_field):


The comment line 382 could probably be removed now

# Handle string for python2 and bytes python3. TODO: lower level fix

pwalczysko

Thank you

will-moore

Looks good, thanks.
Just one TODO to cleanup but not a blocker 👍

will-moore · 2024-03-01T13:20:24Z

omeroweb/custom_forms.py

+                    if value is not None and isinstance(value, str) and len(value) > 0:
+                        value = str(
+                            smart_str(value)
+                        )  # TODO: This is probably a noop now


if isinstance(value, str) then we don't need to do anything - can pass here?

I probably left it in because I wasn't sure that smart_str would not do something

Also worth noting this is a deprecated form (scheduled for removal in a future version) so I would not spend extensive time fixing it.

knabar and others added 14 commits February 6, 2024 14:11

Remove all imports of print_function compatibility method

30907df

Replace all instances of isbytes and bytes_to_native_str

bfbe350

Replace all usages of past.builtins.basestring

2d9b411

Remove usage of past.builtins.long and "import long"

f25db1a

Remove usage of future.standard_library.install_aliases

6a71b74

[pre-commit.ci] auto fixes from pre-commit.com hooks

8fa11fa

for more information, see https://pre-commit.ci

Cleanup miscellaneous Python 2 codepaths

d9f88d2

Remove all unnecessary builtins imports

e8f8ca5

Remove future dependency

913b76b

Clean up outdated Docker infrastructure

5671f8e

Remove Python version check

7db0f55

Remove more Python 2 compatibility code

0c4bc24

Fix flake8

bb3a7f1

Update build matrix by adding 3.12 and removing 3.8

43a887f

knabar force-pushed the maint-python2-cleanup branch from 13115ca to 43a887f Compare February 7, 2024 20:52

Fix flake8

d4368c0

Temporarily point at omero-py GitHub branch with Python 3.12 changes

2edac67

sbesson reviewed Feb 8, 2024

View reviewed changes

omeroweb/settings.py Show resolved Hide resolved

tox.ini Show resolved Hide resolved

knabar added 3 commits February 8, 2024 09:47

Add Python 3.8 back to build matrix

5112ea7

Clean up thread check

ad3ac72

Temporarily require omero-py development branch

52c84a6

knabar force-pushed the maint-python2-cleanup branch from f0c5f6b to 52c84a6 Compare February 8, 2024 15:49

knabar marked this pull request as ready for review February 8, 2024 15:58

sbesson approved these changes Feb 8, 2024

View reviewed changes

sbesson requested review from will-moore, jburel and pwalczysko February 8, 2024 16:37

sbesson mentioned this pull request Feb 21, 2024

Removal of python-future compatibility code ome/omero-py#390

Merged

knabar added 2 commits February 23, 2024 07:06

Set omero-py version

538c256

Remove custom omero-py version from tox

03ca53e

This was referenced Feb 26, 2024

imgData JSON with no stateful services #536

Closed

Img data json django32 #537

Closed

knabar added this to the 5.25.0 milestone Feb 26, 2024

Fix missing build matrix entry

7ddf814

jburel reviewed Feb 27, 2024

View reviewed changes

Remove comment

60ce356

knabar requested a review from jburel March 1, 2024 12:56

pwalczysko approved these changes Mar 1, 2024

View reviewed changes

will-moore approved these changes Mar 1, 2024

View reviewed changes

knabar merged commit 4d8d25e into ome:master Mar 1, 2024
10 checks passed

knabar deleted the maint-python2-cleanup branch March 11, 2024 13:54

will-moore mentioned this pull request Mar 12, 2024

Remove python-future compatibility code #528

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removal of python-future compatibility code #531

Removal of python-future compatibility code #531

knabar commented Feb 6, 2024

knabar commented Feb 7, 2024

sbesson left a comment

sbesson left a comment

jburel commented Feb 9, 2024

sbesson commented Feb 9, 2024 •

edited

Loading

jburel commented Feb 9, 2024

jburel commented Feb 9, 2024 •

edited

Loading

jburel commented Feb 9, 2024

jburel commented Feb 9, 2024

snoopycrimecop commented Feb 10, 2024

snoopycrimecop commented Feb 10, 2024

snoopycrimecop commented Feb 11, 2024

snoopycrimecop commented Feb 11, 2024

snoopycrimecop commented Feb 12, 2024 •

edited

Loading

jburel commented Feb 27, 2024

knabar commented Feb 27, 2024

jburel Feb 27, 2024

pwalczysko left a comment

will-moore left a comment

will-moore Mar 1, 2024

knabar Mar 1, 2024

sbesson Mar 1, 2024

		@@ -296,8 +295,8 @@ def set_if(k, v, func=lambda a: a is not None):

		def decode(shape_field):

Removal of python-future compatibility code #531

Removal of python-future compatibility code #531

Conversation

knabar commented Feb 6, 2024

knabar commented Feb 7, 2024

sbesson left a comment

Choose a reason for hiding this comment

sbesson left a comment

Choose a reason for hiding this comment

jburel commented Feb 9, 2024

sbesson commented Feb 9, 2024 • edited Loading

jburel commented Feb 9, 2024

jburel commented Feb 9, 2024 • edited Loading

jburel commented Feb 9, 2024

jburel commented Feb 9, 2024

snoopycrimecop commented Feb 10, 2024

snoopycrimecop commented Feb 10, 2024

snoopycrimecop commented Feb 11, 2024

snoopycrimecop commented Feb 11, 2024

snoopycrimecop commented Feb 12, 2024 • edited Loading

jburel commented Feb 27, 2024

knabar commented Feb 27, 2024

jburel Feb 27, 2024

Choose a reason for hiding this comment

pwalczysko left a comment

Choose a reason for hiding this comment

will-moore left a comment

Choose a reason for hiding this comment

will-moore Mar 1, 2024

Choose a reason for hiding this comment

knabar Mar 1, 2024

Choose a reason for hiding this comment

sbesson Mar 1, 2024

Choose a reason for hiding this comment

sbesson commented Feb 9, 2024 •

edited

Loading

jburel commented Feb 9, 2024 •

edited

Loading

snoopycrimecop commented Feb 12, 2024 •

edited

Loading