calculate centroid before using to_crs() #165
Conversation
|
ok to merge this @JackKelly ? |
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| rename_save_columns = { |
There was a problem hiding this comment.
Maybe in another PR, we could make it so we don't have to rename the columns, but instead we use the same column names throughout the code?
There was a problem hiding this comment.
Yea I did this as the column names seemed to be limited to 10, but let me check this again
There was a problem hiding this comment.
https://en.wikipedia.org/wiki/Shapefile#Limitations and geopandas/geopandas#1417
says its limited to 10
There was a problem hiding this comment.
And then its a balance of a bit of extra code here to give full verbose, compared to a shorter column name and it not being as verbose. Personally I like the fully verbose version
There was a problem hiding this comment.
Ah, sorry, my mistake... I hadn't realised shapefiles have these limitations! Thanks for explaining. Cool, let's leave the code as-is!
| ) | ||
|
|
||
| # project to WGS84 | ||
| shape_gpd = shape_gpd.to_crs(WGS84_CRS) |
There was a problem hiding this comment.
Is this still necessary? I think the satellite data and the NWPs are in OSGB projection in the batches, right? It'd be good to keep all the geospatial data in the same projection in the batches. Or have I misunderstood?!
| locations_x, locations_y = gsp.get_locations_for_batch(t0_datetimes=gsp.gsp_power.index[0:10]) | ||
|
|
||
| assert len(locations_x) == len(locations_y) | ||
| assert locations_x[0] > 90 # this makes sure it is not in lat/lon |
There was a problem hiding this comment.
OSGB coordinates can be less than or equal to 90 :) I'm not sure of any perfect way to assert that a coordinate is OSGB or lat,lon. Other than, maybe, checking that it's within a certain geospatial boundary (e.g. is it on land within the UK?) But that's maybe overkill?!
There was a problem hiding this comment.
hmm, good point, I think if it was less than 90 it would be in the sea - and we shouldnt have a GSP centroid there, so I'm happy to keep it like this - perhaps Ill add a comment in the code?
There was a problem hiding this comment.
Sure, sounds good: Leave the code as-it but add a comment to explain...
2 participants
Pull Request
Description
Removed warning about
"UserWarning: Geometry is in a geographic CRS. Results from 'centroid' are likely incorrect."
Fixes issue #154
How Has This Been Tested?
normal unittests
added small change to unitest
run ploting script
No
Yes
Checklist: