TheBB commented 8 years ago

It was deemed “out of scope” in https://github.com/Turbo87/utm/issues/18 but it's actually very simple to do and doubtless a useful feature. For people like me who are working with heightmaps with thousands of points in each direction in different coordinate systems, converting coordinate pairs one-by-one is unacceptably slow.

In principle, the numpy module implements all the functions that are needed from math, and they work transparently for arrays as well as floats, so it's a drop-in replacement.

Exceptions:

Bounds checking, I delegated this to a helper function that catches all cases.
Possibly ambiguous input in the zone calculation in from_latlon. Here I just use the first point to compute the zone, assuming that the user is careful enough to have all points in the same zone (or that they will be fine with having input points in one zone converted to UTM coordinates in another).
Mixed north/south latitudes in from_latlon. Right now the code throws an exception in this case. However now that I think about it, I assume it would be correct to add 10e6 northing to every point with a southern latitude and to not change the northern latitude points? If so I can modify this to work without throwing an error.

I wrote the tests to require numpy since it's safer. Not sure what Travis will think of this so let's see…

I'm already using this modified version in my code so it's not critical to me that this is merged, but I'm fairly sure this would be useful to others.

Turbo87 commented 8 years ago

@TheBB nice work! I'm wondering if we could adjust the TravisCI config to test both cases though to make sure one doesn't break the other without noticing.

taylorsansom commented 7 years ago

@TheBB this is great! I just blasted through a conversion of about 80 million points in a matter of seconds. This saved me days if not weeks of processing time!

Turbo87 commented 7 years ago

@TheBB I'd really like to merge this, but we need to be testing both code paths, otherwise I can't accept this PR...

TheBB commented 7 years ago

Yes, I saw that. I'm not sure how best to implement such a thing.

astrojuanlu commented 7 years ago

If there is still interest on this, I can work on the .travis.yml file to add two cases for each Python version: with and without NumPy. The tests are already very quick so there should be no problem. What do you think?

Turbo87 commented 7 years ago

@Juanlu001 sounds good, thanks for looking into this! :)

codecov[bot] commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@4c7c13f). Click here to learn what that means. The diff coverage is 95.91%.

@@           Coverage Diff            @@
##             master     #24   +/-   ##
========================================
  Coverage          ?   93.5%           
========================================
  Files             ?       3           
  Lines             ?     154           
  Branches          ?       0           
========================================
  Hits              ?     144           
  Misses            ?      10           
  Partials          ?       0

Impacted Files	Coverage Δ
utm/conversion.py	`93.33% <95.91%> (ø)`

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 4c7c13f...235bc6b. Read the comment docs.

astrojuanlu commented 7 years ago

The tests finally passed! There is no report to comparate against because we just introduced it :) So @Turbo87, ready for review!

TheBB commented 7 years ago

Thanks @Juanlu001. I actually just pushed to your branch before I saw you close it, but we can use this one too.

danni commented 6 years ago

What has happened to this? It would be nice to crunch all of my values in one go.

astrojuanlu commented 6 years ago

I think it's only missing a review by @Turbo87, the functionality and tests are there!

Turbo87 commented 6 years ago

@TheBB @Juanlu001 sorry, for taking so long. I don't use numpy and currently do very little Python myself, but I've just sent you an invite to the repo so you can help maintain the project if you want to. feel free to merge this PR if you think it's ready :)

if you want to release a new version you only need to update the changelog, bump the version and push a tag to GitHub, and Travis CI will then take care of publishing the package to PyPI.

astrojuanlu commented 6 years ago

Thanks for the invitation @Turbo87, I hope I will do a good job as a maintainer :)

I will proceed and merge this, and hopefully we can prepare a release this week or the following one.

astrojuanlu commented 6 years ago

(I restarted the build just in case, since it's been a while)

astrojuanlu commented 6 years ago

And by "this week or the following one" I actually meant... Next one :crossed_fingers:

Turbo87 / utm

Implement numpy support #24

Codecov Report