Turbo87 / utm

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

support using negative northing utm projection #113

Open saty9 opened 7 months ago

saty9 commented 7 months ago

This allows converting coordinates with points on both sides of the equator and into a continuous space instead of the discontinuity from adding 10,000,000 to southern equator points

bartvanandel commented 7 months ago

I'm not sure if this is the right solution to the problem you're facing. Could you explain why you'd need this added to the library?

Those checks exist for a reason: this method can handle either a single lat/lng pair, or a whole list of them. In the latter case, it uses the first pair to determine the zone number and letter. Note that this relies on the combination of latitude and longitude, so mixing lat/lng pairs that fall into various zones will result in erroneous output, without warning (apart from the latitude sign checks).

There are a few other issues with the changes, e.g. the name of the new option doesn't really reflect what it's supposed to do (basically, skip the latitude sign check). The parameter isn't documented in the function doc, and the return logic is duplicated, which can be easily avoided by something like this:

if not skip_sign_checks:  # note: different name which more clearly indicates what it does
  if mixed_signs(...):
    ...
  elif negative(...);
    ...

But again, I have a feeling this isn't the right approach.

saty9 commented 7 months ago

I need it as I anticipate having some data that crosses the equator. We are using the library to covert GPS data so it can be plotted on a scatter graph at the moment as our use case is for small areas (think sports pitches size) the distortion as you cross zones should be minimal as we don't go very far past zone borders. Plenty of people live near the equator so a path that crosses it is not unlikely. The objective was both to skip the mixed sign check and to produce a continuous coordinate space instead of what you get at the moment if you did points one at a time where points below (south of) the equator are moved to above points above (north of) the equator.

I don't think skip sign checks is correct as a name because of that property. Agree with your points about doc strings though but will hold off on that until there is a more positive view about the main idea.

bartvanandel commented 7 months ago

Thanks for providing some clarification, I understand your use case now.

For some more context: do you envision always skipping this negative value conversion, or only when your data set is very close to the equator? I.e., when the data set is, say, 1000km south of the equator, do you still want to skip the conversion?

We need a better name though, because this option doesn't just affect (single) negative values, it also skips validation for mixed positive and negative values which is normally a good thing. Maybe keep_negative_values would do the trick, with proper documentation.

Another approach could be to define actions for negative values and mixed values, so the behavior is configurable for both cases separately (default: raise error). But this complicates things even more.

bartvanandel commented 7 months ago

I just noticed another PR from some time ago (Aug 2021) that addresses this exact issue in very nice way: #46, which is also referred to from #94. Looks like the maintainer may be a bit short on time :(

saty9 commented 7 months ago

For our use case I would envision just leaving the flag on all the time as the behaviour without it for coordinates to the south is just to add 10,000,000 which at best will do nothing to accuracy and at worse harm it thanks to floating point.

What about negative_southern_hemisphere or continuous_equator or continuous_northing

Edit: Had a look at #46 yes that approach would work I think

bartvanandel commented 7 months ago

Now if only we could get @Turbo87 on board. I'm afraid I'm not a maintainer (for some reason I thought I was)

bartvanandel commented 7 months ago

Had a look at #46 yes that approach would work I think

I've just merged that one yesterday. There's no release yet, I'd like to validate a few things before proposing that, but could you check if the merged code does the trick for you?

saty9 commented 7 months ago

46 is still open it hasn't been merged?

86 would have helped if i were going in the other direction.

bartvanandel commented 7 months ago

Oops, you're right, must be Friday today...

86 doesn't actually introduce new math, it just prevents in-place modification of the y value when using numpy.#113

The reason I haven't merged #46 yet is because it removes the (everything) negative and mixed signs checks entirely, and I'm not 100% convinced they shouldn't exist in some way. OTOH there are no checks preventing entering a dataset that spans the entire globe in the longitudinal (east-west) direction either, so those validations are not covering much ground anyway.

If you can provide any advice or opinion on that, much appreciated. I'm not currently much of a user of this library myself, to be honest, I just like geo.

sdhiscocks commented 7 months ago

@bartvanandel If you need any changes to #46 or have any concerns, happy to take a look. Would be good to get this fixed.

it removes the (everything) negative and mixed signs checks entirely, and I'm not 100% convinced they shouldn't exist in some way

The 10,000,000 offset that's applied for southern hemisphere is only done to avoid negative numbers, so should be safe to remove.

And thanks for your recent activity on this library 👍

bartvanandel commented 7 months ago

The 10,000,000 offset that's applied for southern hemisphere is only done to avoid negative numbers, so should be safe to remove. I know, but as I understand this, it's part of the UTM coordinate convention, see e.g. https://www.maptools.com/tutorials/utm/details. Also, the PR doesn't remove that offset, it just switches using the negative function with a condition based on the zone letter. What it does remove is any checks to make sure all points are on the same side of the equator.

I'm actually thinking about adding an argument named something like assume_same_zone = True to address the problem of having points (possibly) all over the place differently. There are several issue reports where people are a bit confused about the current behavior of having the first point dictate the zone. But that's another issue.

And thanks for your recent activity on this library 👍 My pleasure!

sdhiscocks commented 7 months ago

What it does remove is any checks to make sure all points are on the same side of the equator.

Sure, there is no need to ensure that points are on the same side of the equator. Just that the offset is applied or not, based on the hemisphere of the zone selected; or for @saty9's use case, just let negative numbers for southern hemisphere.

heathhenley commented 4 months ago

As far as I understand, changing to allow negatives instead of starting the equator at 10,000,000 meters northings would not align with the definition of UTM (https://pubs.usgs.gov/pp/1395/report.pdf, end of second paragraph on page 58). It is only done to avoid negatives as you said, but it's also in the definition. So comparing between two different systems wouldn't make sense (I have a use case for this between python and this module, and a separate implementation in c++, for example).

I get that you made it conditional and default to current behavior though, so that's cool. I wonder if this is something that is common in other libraries to convert to UTM (Proj, etc), and if there are others interested in this feature.

For your plotting use case - maybe you could wrap the utm call and correct nothings when below the equator? I'm actually doing something similar to avoid the issue in #86 without changing the client code because there's not been a release pushed to pypi in a while.

sdhiscocks commented 4 months ago

I get that you made it conditional and default to current behavior though, so that's cool. I wonder if this is something that is common in other libraries to convert to UTM (Proj, etc), and if there are others interested in this feature.

Supported from for example pyproj.

In [1]: from pyproj import Transformer

In [2]: transformer = Transformer.from_crs(4326, 32631)

In [3]: transformer
Out[3]: 
<Conversion Transformer: pipeline>
Description: UTM zone 31N
Area of Use:
- name: Between 0°E and 6°E, northern hemisphere between equator and 84°N, onshore and offshore.
- bounds: (0.0, 0.0, 6.0, 84.0)

In [4]: transformer.transform(-1, 0)
Out[4]: (166072.0629036766, -110682.83775879144)

In [5]: transformer.transform(1, 0)
Out[5]: (166072.0629036766, 110682.83775879144)

Without some support for mixed signs, this library currently can't easily be used for any data that spans the equator.

heathhenley commented 4 months ago

That makes sense, thanks. I guess I didn't understand that you wanted to force a southern point to a northern zone:

>>> from pyproj import Transformer
>>> import utm
>>> to_31n = Transformer.from_crs(4326, 32631)
>>> to_31s = Transformer.from_crs(4326, 32731)
>>> utm.from_latlon(-1, 0)
(166072.06300212978, 9889317.162242597, 31, 'M')
>>> to_31n.transform(-1, 0)
(166072.0629036766, -110682.83775879144)
>>> to_31s.transform(-1, 0)
(166072.0629036766, 9889317.16224121)

Probably makes sense to have more control here, like you would with proj - IMO I agree that #46 looks like a great approach. I should have commented there originally....

@bartvanandel Is there anything we could help with to get a release out with all these recent updates?