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

GeoToH3 filter not picking up correct argument on Mac OS cli #39

Closed
thomasgwatson opened this issue Apr 2, 2018 · 6 comments
Closed

Comments

@thomasgwatson
Copy link

Followed the instructions to build H3 from source on Mac OS, including sudo make install for global install.

Tried testing it with the example call:

geoToH3 10 40.689167 -74.044444

got back

usage: geoToH3 resolution

Looking at https://github.com/uber/h3/blob/master/src/apps/filters/geoToH3.c
lines 43 to 46:

seems like its printing the error message and then exiting if there are not 2 command line arguments... but aren't there meant to be 4?

Also, after adjusting that it started hanging in the while loop starting on line 60.

I'm not familiar with C, so debugging is slow going but will resume tomorrow.

However it does feel like some sort of weird compiler incompatibly.

@dfellis
Copy link
Collaborator

dfellis commented Apr 2, 2018

Hi @thomasgwatson where are you seeing that example call? Because that's not correct.

You invoke geoToH3 with the H3 resolution you're interested, eg geoToH3 10 and then it starts and waits for user input for the lat and lng coordinates separated by newlines.

damocles@Talyn:~/oss/h3(master)$ ./bin/geoToH3 10
40.689167 -74.044444
8a2a1072b59ffff
^C

Like that. If you want a single command that takes all of the arguments up front, prints, and then exits, you have to type a slightly more awkward command involving echo and a pipe.

damocles@Talyn:~/oss/h3(master)$ echo "40.689167 -74.044444" | ./bin/geoToH3 10
8a2a1072b59ffff

@thomasgwatson
Copy link
Author

Ahh ok. I see. I just assumed it was a one liner.

Assuming it was built this way for performance reasons? That its faster to just throw a bunch of pairs on to stdin than have separate calls?

Thanks for getting back to me!

@dfellis
Copy link
Collaborator

dfellis commented Apr 3, 2018

The tool was originally built by @sahrk for debugging purposes, I believe.

We've debated switching to something like getopt but then there's a split between GNU getopt and POSIX-style where GNU does the more useful thing, but then there's issues of cross-compatibility, licensing, and the added complication of making this code have another dependency in a language that doesn't handle dependencies (at all) for you.

In the end, we expect most people to use the bindings in other higher-level languages (whenever they get released. Hi, @isaacbrodsky ;) ), or use the code directly in their own C/C++ project, and these command-line utilities to get very little real-world usage, so the slightly unfamiliar usage pattern was considered "okay," but not great.

@sahrk
Copy link
Collaborator

sahrk commented Apr 3, 2018

Yes, I wrote the guts of several of the applications in src/apps/filters while debugging and testing the core C functions, and the command line argument handling in those is primitive. But I would argue in defense of the usefulness of traditional Unix command-line filters that operate on text streams. For example, to transform a text file containing a sequence of latitude/longitudes into H3 indexes is as simple as geoToH3 8 < geo.txt > h3.txt. And the filter applications can be added to pipelines with other filters to perform more complex reference frame transformations.

@dfellis
Copy link
Collaborator

dfellis commented Apr 3, 2018

I wasn't arguing against that @sahrk ! Going with a stream-based API from the get-go let's you work around a lack of single lat, lng request with just an echo and pipe.

GNU has simply made it common practice for commands to build in support for easily doing the top N tasks a tool can be used for, so we'll probably add that support at some point.

@thomasgwatson
Copy link
Author

Thanks for providing so much context, really appreciate it!

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

No branches or pull requests

4 participants