Turbo87 / utm

Bidirectional UTM-WGS84 converter for python
http://pypi.python.org/pypi/utm
MIT License
486 stars 101 forks source link

Make sure numpy northing array is not modified in place in `to_latlon` #86

Closed heathhenley closed 7 months ago

heathhenley commented 2 years ago

We were using these eastings / northings (in np.arrays) after calling the to_latlon function and noticed that the northing was being modified. I'm not sure if this is actually intended to be supported in UTM, but I don't think it should be modified as an arg. It seems like the -= operator here was optimized to modify the northings in place, which was a problem for us. Given that the easting is not modified in place, I think it make sense to do it this way instead.

heathhenley commented 9 months ago

Hey @Turbo87 - sorry for the ping, I was just looking through old open stuff I'm on and this came up. Do you think it's suitable to merge or better to close?

bartvanandel commented 7 months ago

Maybe to prevent this from being "optimized" into -= in the future (as it isn't necessarily trivial why it should be avoided), maybe go for this?

y = northing if northern else northing - 10000000
heathhenley commented 7 months ago

That works too, maybe adding a comment if you think it's likely to get refactored again?

heathhenley commented 7 months ago

Happy add and rebase - this is pretty old so probably behind

bartvanandel commented 7 months ago

It's still a clean merge, so it's not that outdated. Checks may fail (unrelated to this PR) due to https://github.com/Turbo87/utm/pull/103 though.

heathhenley commented 7 months ago

Ok it's inlined now, I didn't pull down and test though, lmk if you think it's needed.

bartvanandel commented 7 months ago

I can't merge, but @Turbo87 can?