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

handle scipy deprecation #91

Merged
merged 2 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/envs/38-minimal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ channels:
- conda-forge
dependencies:
- python=3.8
- scipy=1.7
- scipy=1.8
- numpy=1.21
- pandas=1.3
- matplotlib=3.4
Expand Down
2 changes: 1 addition & 1 deletion pointpats/geometry.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

HULL_TYPES = (
numpy.ndarray,
spatial.qhull.ConvexHull,
spatial.ConvexHull,
)

## Define default dispatches and special dispatches without GEOS
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
scipy>=0.11
scipy>=1.8.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
scipy>=1.8.0
scipy>=1.7.0

We have 1.7.4 in the CI and it works as expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Scipy bumped from v1.7.3 to v1.8.0 without having intermediate releases. So I think scipy>=1.8.0 and scipy>=1.7.4 are equivalent and I do not think v1.7.3 or v1.7.1 works.

Copy link
Member

Choose a reason for hiding this comment

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

We have 1.7.3 (typo above, sorry) here https://github.com/pysal/pointpats/actions/runs/4320355325/jobs/7540519213 and it works apparently. So 1.7 seems to be enough now I believe.

Copy link
Member Author

@weikang9009 weikang9009 Mar 3, 2023

Choose a reason for hiding this comment

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

This is testing on scipy v1.10.1?
image

And it seems the deprecations happened in v1.8.0 based on the release note
https://docs.scipy.org/doc/scipy/release.1.8.0.html#clear-split-between-public-and-private-api

Copy link
Member

Choose a reason for hiding this comment

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

Ah! I missed this. And It also means that I have not done #87 properly. The minimal env should use the pinned versions of SciPy. Not sure why it gets upgraded by pip...

Copy link
Member

Choose a reason for hiding this comment

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

Nope, I am stupid. It is the change in this PR that is forcing the update.

If 1.8 is the new minimal, can you also update it in the 38-minimal environment? So we really get 1.8 and not 1.10 when we want to test the oldest supported version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just updated the 38-minimal environment

numpy>=1.3
pandas
matplotlib
Expand Down