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

Scripts and dependencies updates for Python 3.8 - 3.10 #2009

Merged
merged 35 commits into from
Aug 12, 2024

Conversation

shachafl
Copy link
Collaborator

@shachafl shachafl commented Mar 5, 2024

The motivation for this PR was significant algo improvements for blob detection in scikit-image 0.20 released on February 2023, together with some recommendations for the python scientific community about maintaining support for outdated packages and python interpreters (https://scientific-python.org/specs/spec-0000/).

Nevertheless, at this point I only recommend dropping support for python < 3.8, as it seems still very popular, but I don't feel strongly about it if the team prefer to drop support for python < 3.10.

To update dependencies and solve conflicts I used pip-tools (https://github.com/jazzband/pip-tools):

Make new virtual environment and upgrade/install packages

$ python3 -m venv venv
$ source venv/bin/activate
$ python3 -m pip install --upgrade pip setuptools wheel pip-tools

To update dependency list, I removed REQUIREMENTS.txt file, and run (this step is only needed when wishing to change dependency versions, see pip-tools docs):

$ rm REQUIREMENTS.txt
$ pip-compile requirements.in --output-file REQUIREMENTS.txt

Install/update python packages, equivalent to "python3 -m pip install -r REQUIREMENTS.txt"

$ pip-sync REQUIREMENTS.txt

Install starfish from local repository, in development mode:

$ pip3 install -e .

After further updating requirements/REQUIREMENTS-CI.txt.in and using pip-tools again, I was able to install starfish following the developer guide (https://spacetx-starfish.readthedocs.io/en/latest/developer_guide/) and run the tests listed in starfish-prod-ci.yml using the Makefile.

Starting to summarize the PR (by commits):

  1. Changes due to scikit-image ver 0.19-0.21 (not backward compatible): 0f63cd0, 6c4c2df, e21a642, e4e6499 (Expose GeometricTransform.residuals in HTML doc scikit-image/scikit-image#6968), b21f2af.

  2. Upgrading to pandas 2.0 with backward compatibility:
    3653ad7

  3. Napari >= 0.4.18 (& Vispy >= 0.12): dropped support for anisotropic spot size (https://forum.image.sc/t/anisotropic-point-sizes/83388). Here I left most of the code as anisotropic spot size is suppose to return at a later Napari/Vispy version. I only used np.mean() to get a single spot size, but don't feel strongly about it if min(), max(), etc. is preferred. 19b4e00

  4. Updating dependencies in REQUIREMENTS.txt: d5c1835, 68d338d

  5. Deprecated numpy dtypes: 7b24d34

Lastly, this PR will require some minor updates to docs (using pip-tools in Makefile, removing python < 3.8, etc).

@davidhbrann
Copy link

Thanks for this—much appreciated. I was struggling with the various dependency issues between starfish, napari, xarray, numpy and scikit-image over the past few days, but this PR fixes it, and also fixes the various scikit-image deprecation bugs, as well as the changes in napari for spots (I had just been adding the coords for each spot manually to the viewer with .add_points()).

I tried installing everything in a python 3.10 conda environment and it seemed like it was working for me.

@berl
Copy link
Collaborator

berl commented Mar 6, 2024

this is really great- thanks @shachafl for digging into this. Currently the mypy check is failing (to run)- can you check if 0.910 (from below) is really the latest compatible with python=3.8? If not, you can try pinning it to the latest version in the CI requirements and see if it runs?

flake8 --ignore 'E252, E301, E302, E305, E401, W503, E731, F811' --exclude='*__init__.py' starfish examples/data_formatting_examples/format*
flake8 --ignore 'E252, E301, E302, E305, E401, F401, W503, E731, F811' --filename='*__init__.py' starfish examples/data_formatting_examples/format*
mypy --ignore-missing-imports starfish examples/data_formatting_examples/format*
/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/jupyter_client/manager.py: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 0.910
/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/jupyter_client/manager.py: : note: please use --show-traceback to print a traceback when reporting a bug
make: *** [Makefile:60: mypy] Error 2
Error: Process completed with exit code 2.

@shachafl
Copy link
Collaborator Author

shachafl commented Apr 18, 2024

@berl I hit a "wall" when running further tests:
$ make fast-test
Most of the errors are related to "jsonschema.exceptions._RefResolutionError: <urlopen error [Errno 2] No such file or directory:..." as a result of path with an additional schema sub directory - /starfish/spacetx_format/schema/schema/...
I am using jsonschema==4.21.1, and it seems due to updates in version 4.18: python-jsonschema/jsonschema#1119
Rolling back to version 4.17 removes the jsonschema errors with the double schema but gives errors with unnecessary addition to path of "/schema/field_of_view/tiles/":
/starfish/spacetx_format/schema/field_of_view_0.1.0/tiles/schema/field_of_view/tiles/coordinates.json

Any assistance is welcome.

@shachafl
Copy link
Collaborator Author

Almost.
I get the following error when generating html files with Sphinx:
$ make docs-html

reading sources... [100%] user_guide/working_with_starfish_outputs/index

Warning, treated as error:
Failed guarded type import with ImportError("cannot import name 'DatetimeLike' from 'xarray.core.types' (/home/Code/test/starfish/.venv/lib/python3.8/site-packages/xarray/core/types.py)")

I can circumvent the error on my local machine by running the command twice, and between the runs edit docs/source/conf.py to add 'xarray' to autodoc_mock_imports:
autodoc_mock_imports = ['_tkinter', 'xarray']
The reason is that sphinx-gallery doesn't run example python scripts if they haven't changed since the last run.

This can be reproduced locally by forcing sphinx to run all the relevant scripts (adding the below to conf.py):
sphinx_gallery_conf = {
...,
# Rerun stale examples even if their MD5 hash shows that the example did not change.
'run_stale_examples': True,
}

In this case sphinx-gallery fails to run as it is doing a mock import of 'xarray' when running python plot scripts.

@shachafl
Copy link
Collaborator Author

shachafl commented Jun 3, 2024

Hey @berl,
Can you help review or find a reviewer to merge this PR?

Many thanks in advance.

@shachafl shachafl changed the title Scripts and dependencies updates for Python3.8 & 3.10 Scripts and dependencies updates for Python 3.8 - 3.10 Jun 6, 2024
@berl
Copy link
Collaborator

berl commented Jun 6, 2024

@shachafl thanks for grinding through the dependencies on this! also, pip-tools is new to me and looks quite nice. a couple things:

  1. there are some quantitative changes you made in some of the test results. This is not terribly alarming when we consider the likely churn in the underlying image processing packages, but it would be good to show that the new results are still reasonable by eye. Can you visually verify these and post images here?
  2. I get a couple errors running make fast-test on mac OS 12.6.5. I'll see if I can nail them down.

@shachafl
Copy link
Collaborator Author

shachafl commented Jun 7, 2024

@berl thanks for reviewing this. This was a much deeper "rabbit hole" than intended. Regarding your items:

  1. a bug fix for phase_cross_correlation() was issued in scikit-image 0.19 (https://scikit-image.org/docs/stable/release_notes/release_0.19.html). This results in minor differences in the translation transforms and registered_images, as can be seen in the plots below , and some other minor result changes in downstream analysis in the ISS pipeline:

image

  1. This is probably due to pyqt on MacOs (https://forum.image.sc/t/issues-with-installing-starfish/89580/5). Try running pip-compile with python 3.8 and compare the dependency versions you get under MacOs with what I got (WSL2 with Ubuntu 22.04). We might find some common list that works for all.

@shachafl
Copy link
Collaborator Author

This PR closes: #2001 #2000 #1999 #1996 #1995 #1994 #1993 #1992 #1991 #1990 #1989

selsm was replaced with footprint in scikit-image 0.20
…ate all req files

updating requirements files to support sphinx's make html
…e but got ndarray instead: replaced np.arange with range
Copy link
Collaborator

@berl berl left a comment

Choose a reason for hiding this comment

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

super excited about this. there are a few places we should note planned improvements:

  • xarray pinning is (I think) due to some outdated syntax for initializing a DataArray somewhere in ImageStack
  • napari issues date back to before the days of napari-plugin so there may be a better way to include napari for visualization by going that route
  • on the topic of visualization, SpatialData uses an external commercial viewer (10x Xenium Explorer) by writing a compatible file bundle. There are some risks with this (there is still quite a bit of churn on the viewer and the file spec and I'm not aware of a validator other than trying to open a file) but it would bring starfish output into the modern world.

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