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

warn for indices that expect certain projections #96

Closed
knaaptime opened this issue Jul 2, 2019 · 13 comments
Closed

warn for indices that expect certain projections #96

knaaptime opened this issue Jul 2, 2019 · 13 comments

Comments

@knaaptime
Copy link
Member

DistanceDecayExposure for example, expects wgs84. We decided against implicit reprojection, but we could do a check and raise a warning if we cant determine the crs is wgs84

@renanxcortes
Copy link
Collaborator

Hm.. I'm not sure if I follow... this expectation is due to the usage of euclidean_distances in line

dist = euclidean_distances(
?
BTW, I think that we can improve these distances with the proper projections and so on... working with geodesic distance or something like this!

@knaaptime
Copy link
Member Author

image

the original CRS is albers equal area, and if you try to compute DDE, you get 0. If you reproject into wgs84, you get 0.23

we might just check whether the crs is epsg4236 and warn if not. But agree, a more flexible handling of distance calculation would be prefereable

@ljwolf
Copy link
Member

ljwolf commented Jul 2, 2019

a ‘metric’ argument might be useful... that’d let you specify the right metric, and use haversine if the coordinates are in lat/lng

@renanxcortes
Copy link
Collaborator

a ‘metric’ argument might be useful... that’d let you specify the right metric, and use haversine if the coordinates are in lat/lng

I was thinking about something like this. Would you have some example somewhere that I could draw inspiration from? :)

@ljwolf
Copy link
Member

ljwolf commented Jul 2, 2019

This would be a good ref.

https://github.com/scipy/scipy/blob/v1.3.0/scipy/spatial/distance.py#L1736

I'd do euclidean, unless someone sends a crs with a units field in degrees?

@renanxcortes
Copy link
Collaborator

This would be a good ref.

https://github.com/scipy/scipy/blob/v1.3.0/scipy/spatial/distance.py#L1736

I'd do euclidean, unless someone sends a crs with a units field in degrees?

Thanks! Ok, but what about the above aea example that the units is in m? Shouldn't the haversine be applied to that case?

renanxcortes added a commit to renanxcortes/segregation that referenced this issue Jul 2, 2019
…d metrics

This address pysal#96

ps.: it also makes a small tweak on formating of spatial proximity profile
@knaaptime
Copy link
Member Author

na its good, the meters means its already projected into a plane

@renanxcortes
Copy link
Collaborator

So, just for registration, the first example using the old only euclidean based measures was returning zero because of the magnitudes of the number of the albers equal projection. These are the centroids number:

image

which leads to these numbers after:

c = np.exp(-dist)
Pij = np.multiply(c, t) / np.sum(np.multiply(c, t), axis=1)
DDxPy = (x / X * np.nansum(np.multiply(Pij, y / t), axis=1)).sum()

image

Since DistanceDecayExposure uses np.nansum the resulting value was zero. I think if we rerun with this new haversine metric, the resulting value will make more sense.

@renanxcortes
Copy link
Collaborator

Ok, after discussing here with @knaaptime , things got clearer for me. So, actually using the haversine distance with albers projection is not correct since this crs is already projected to a planar plane. So, what we could do in this case is: if any problem due to the magnitude of the number occurs and generates nan in that distance matrix, raise a ValueError to the user.

Besides that, assuming that we'd like to help our users to use these functions, we need to create a bunch of specific tests using the .crs attribute of the input data. The idea is: if we "suspect" that the user is working with spherical coordinates, raise a warning saying: "It seems like you're working with unprojected coordinate system. Therefore, we recommend to set metric = haversine." but in any case calculate the index, because these tests might not capture different funky ways that the .crs might be. Did I get it right, @knaaptime?

renanxcortes added a commit to renanxcortes/segregation that referenced this issue Jul 3, 2019
@renanxcortes
Copy link
Collaborator

So, this is partially solved due to #100. However, I still think that a set of tests/conditions for the input of the GeoPandas to let the user aware of it would be useful. However, now the user can choose between euclidean or haversine distances but it is up to him to know the nature of his GeoDataFrame.

@knaaptime
Copy link
Member Author

with the new pyproj integrated into geopandas, it should be much easier to create checks for CRS versions. Re-upping this thread to remind myself to revisit and rewrite those checks

@knaaptime
Copy link
Member Author

this is breaking, e.g. compute_segregation_profile because it tries to introspect the .crs attribute with dict notation, but the CRS object isnt a dict anymore

@knaaptime
Copy link
Member Author

geopandas has better CRS handling now (warnings for mismatches and warning when using distance with geometric proj) so closing this here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants