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

ENH: use pyproj.CRS when possible #998

Closed
wants to merge 2 commits into from

Conversation

snowman2
Copy link
Contributor

Needs tests for new version & probably needs current tests fixed.
Much to do here, but it is a start.

@snowman2
Copy link
Contributor Author

snowman2 commented May 19, 2019

Likely will address (but remains to be seen):
#943
#245
#65

@snowman2
Copy link
Contributor Author

Lots of pyproj version checking going on. Would you prefer to just pin pyproj>=2.2.0?

@snowman2 snowman2 force-pushed the class_crs branch 3 times, most recently from 1cff977 to 19d335c Compare May 19, 2019 01:27
@codecov
Copy link

codecov bot commented May 19, 2019

Codecov Report

Merging #998 into master will decrease coverage by 1.82%.
The diff coverage is 70.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #998      +/-   ##
==========================================
- Coverage   89.22%   87.39%   -1.83%     
==========================================
  Files          18       18              
  Lines        1642     1674      +32     
==========================================
- Hits         1465     1463       -2     
- Misses        177      211      +34
Impacted Files Coverage Δ
geopandas/geoseries.py 93.33% <60%> (-5.96%) ⬇️
geopandas/io/file.py 83.65% <72.72%> (-11.09%) ⬇️
geopandas/tools/crs.py 82.35% <77.77%> (-2.84%) ⬇️
geopandas/base.py 93.96% <85.71%> (-0.31%) ⬇️
geopandas/io/sql.py 78.57% <0%> (-17.86%) ⬇️
geopandas/geodataframe.py 96.84% <0%> (-0.91%) ⬇️
geopandas/plotting.py 91.73% <0%> (-0.87%) ⬇️
geopandas/tools/overlay.py 55.72% <0%> (-0.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01f96f6...19d335c. Read the comment docs.

@codecov
Copy link

codecov bot commented May 19, 2019

Codecov Report

Merging #998 into master will decrease coverage by 1.85%.
The diff coverage is 65.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #998      +/-   ##
==========================================
- Coverage   89.72%   87.86%   -1.86%     
==========================================
  Files          20       20              
  Lines        1936     1978      +42     
==========================================
+ Hits         1737     1738       +1     
- Misses        199      240      +41
Impacted Files Coverage Δ
geopandas/tools/sjoin.py 96.92% <100%> (ø) ⬆️
geopandas/base.py 90.62% <54.54%> (-3.93%) ⬇️
geopandas/geoseries.py 89.71% <62.5%> (-4.3%) ⬇️
geopandas/tools/crs.py 82.35% <77.77%> (-2.84%) ⬇️
geopandas/io/file.py 90% <85.71%> (-4.74%) ⬇️
geopandas/io/sql.py 82.14% <0%> (-14.29%) ⬇️
geopandas/_compat.py 85.71% <0%> (-14.29%) ⬇️
geopandas/tools/_show_versions.py 88.31% <0%> (-2.6%) ⬇️
geopandas/plotting.py 91.17% <0%> (-2.11%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29add0a...1b58545. Read the comment docs.

@snowman2 snowman2 force-pushed the class_crs branch 5 times, most recently from 94378a7 to 20b141c Compare May 20, 2019 01:08
@snowman2
Copy link
Contributor Author

One issue I see is that PROJ does not consider epsg:4326 and +init=epsg:4326 to be equal, whereas fiona does:

>>> crs_wkt = 'GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4326"]]'
>>> cc1 = CRS(crs_wkt)
>>> cc1
<Geographic 2D CRS: EPSG:4326>
Name: WGS 84
Axis Info [ellipsoidal]:
- Lat[north]: Geodetic latitude (degree)
- Lon[east]: Geodetic longitude (degree)
Area of Use:
- undefined
Datum: World Geodetic System 1984
- Ellipsoid: WGS 84
- Prime Meridian: Greenwich

>>> cc2 = CRS("+init=epsg:4326 +type=crs")
>>> cc2
<Geographic 2D CRS: +init=epsg:4326 +type=crs>
Name: WGS 84
Axis Info [ellipsoidal]:
- lon[east]: Longitude (degree)
- lat[north]: Latitude (degree)
Area of Use:
- name: World
- bounds: (-180.0, -90.0, 180.0, 90.0)
Datum: World Geodetic System 1984
- Ellipsoid: WGS 84
- Prime Meridian: Greenwich

The difference is subtle, but it is mostly the difference between the axis being geodetic or not.
This is an issue when writing to a file and reading in the projection back in.

@jorisvandenbossche
Copy link
Member

@snowman2 Thanks a lot for starting this!

I opened #1003 for some of the more general discussion points. Will try to give more detailed code feedback here later.

@snowman2
Copy link
Contributor Author

Not sure where it is coming from but I don't see epsg:26018 on any google search or on:
http://epsg.io/?q=26018.
And it seems PROJ does not recognize it either and is causing tests to fail.

@snowman2
Copy link
Contributor Author

Using pyproj 2.2.0 (current master):
image

@snowman2 snowman2 force-pushed the class_crs branch 4 times, most recently from 77c45ee to 99fa0b8 Compare May 23, 2019 23:49
@snowman2 snowman2 changed the title WIP: updated to use pyproj.CRS when possible ENH: updated to use pyproj.CRS when possible May 23, 2019
@snowman2 snowman2 changed the title ENH: updated to use pyproj.CRS when possible ENH: use pyproj.CRS when possible May 23, 2019
@snowman2 snowman2 force-pushed the class_crs branch 2 times, most recently from 1b56d90 to ea34102 Compare May 24, 2019 00:06
@snowman2
Copy link
Contributor Author

snowman2 commented Jul 4, 2019

I came up with a version that returns a dictionary. Only one test on Travis failed due to precision on python 2. The changes are on a separate commit and can be easily removed depending on the direction you would like to proceed in. If you like the dict option, I can fix that python 2 test to require less precision when testing the difference.

@snowman2
Copy link
Contributor Author

snowman2 commented Sep 21, 2019

Closing for preference to #1101

@snowman2 snowman2 closed this Sep 21, 2019
@snowman2 snowman2 deleted the class_crs branch August 4, 2022 17:17
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