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

Get rid of pyproj.FutureWarning #216

Merged
merged 5 commits into from
Apr 25, 2023

Conversation

philippkraft
Copy link
Contributor

#210 is a about a future warning of pyproj. This pull request handles it for sgrid and sview - not pgrid / pview. All calls to pyproj in sgrid, sview and io are refactored into a new file projection.py which handles the different APIs of pyproj and exposes a consistent API to sgrid / sview / io, which resambles the 2.2 to 2.6 API of pyproj. This can be used for pgrid / pview, but I do not understand the functionality of pgrid and pview. test_grid.py passes.

distutiles.version.LooseVersion is also deprecated in Python 3.10 and is replaced in projection.py with packaging.Version.

@mdbartos
Copy link
Owner

Looks like tests are still failing.

See also #214 (comment)

Thanks,
MDB

@philippkraft
Copy link
Contributor Author

Ok, the test was working on my system, I will investigate the problem here, but I will need some time.

Philipp Kraft and others added 2 commits March 16, 2023 10:20
@philippkraft
Copy link
Contributor Author

Found the problem: test_grid.py runs now "really" on my system, and I solved the bug. Underlying issue: pyproj changed around Version 2 the semantics of the words "Projection" and "CRS". The code (my new one and your code) adds some confusion (at least I was completely confused). I would suppose to leave this as is for now, but as soon as you are fine to drop support for pyproj < 3, I can clean up the mess in projection.py.

@mdbartos
Copy link
Owner

I would be OK dropping support for pyproj < 3 if it is time. I would want to create a new version and release though if incorporating breaking changes.

With geospatial python I am always a bit hesitant, because I know that on certain systems I have had to revert to old versions of GDAL/Proj4/etc to get things working.

@philippkraft
Copy link
Contributor Author

If you merge this PR we get rid of the annoying warning with the "new" pyproj, but still have full compatibility with even pyproj<2.1 (03/2019). If (whenever) you start a branch for a new version with dropping support on pyproj < 3 (from 11/2020), you can ping me and I will clean up the messy parts (compatibility layers) of projection.py.

@mdbartos
Copy link
Owner

Hi @philippkraft, let me know when this is ready to pull in. I'll create an issue reminding me to pull in a cleaned-up version at a later date when pyproj 2 can be deprecated with certainty. If possible, could you create another pull request on a different branch with the cleaned-up version?

@mdbartos mdbartos mentioned this pull request Mar 24, 2023
@philippkraft
Copy link
Contributor Author

philippkraft commented Mar 24, 2023

I think it is ready to merge.

For the future version: If you create a fitting branch, I can create the PR against that branch. However cleaning this up will diverge pgrid and sgrid even more. I think I read in some issue, you are about to retire pgrid.py, perhaps the deprecation of pyproj2 would come together with retiring pgrid.py, if you plan it?

UPDATE: I still need to do some tests!

@mdbartos
Copy link
Owner

Hi @philippkraft, just checking if this is ready to pull in. You mentioned needing to do some tests in your last comment.

I've slated retiring pgrid as a to-do #221.

@philippkraft
Copy link
Contributor Author

I checked it and I am working currently with the branch. You can pull it in. And I will subscribe to #221 and see when I can contribute the shortened version of projection.py

@mdbartos mdbartos merged commit 63788bc into mdbartos:master Apr 25, 2023
@philippkraft philippkraft deleted the i210_pyproj_future_warning branch April 27, 2023 18:41
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.

2 participants