When using the latest published crate 0.2.3 building the example fails complaining that there are no functions encode or decode. Using geohash = { git = "https://github.com/georust/rust-geohash.git" } works fine. So I suspect a new version simply needs to be published to cargo?

When cross-building in OSX to linux-musl:

--> /cargo/registry/src/github.com-1ecc6299db9ec823/geohash-0.12.0/src/core.rs:88:57 | 88 | pub fn encode(c: Coordinate<f64>, len: usize) -> Result<&String, GeohashError> { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments help: consider using the'staticlifetime | 88 | pub fn encode(c: Coordinate<f64>, len: usize) -> Result<&'static String, GeohashError> { | ~~~~~~~~

CMD: cross build --target aarch64-unknown-linux-musl

rustc 1.59.0 (9d1b2106e 2022-02-23)

I don't know if I'm doing something wrong, but upon importing the crate geo_types and try and use geohash I get this error when trying to use the encode function.

expected struct geo::types::Coordinate, found struct geo_types::Coordinate

Would you consider re-exporting Coordinate? The way things stand now, users of this crate also need to explicitly import geo.

(Edit: in which case it's down to the user to explicitly co-ordinate the versions of Coordinate between geo, and geohash: using geohash with geo 0.8.0 doesn't work.)

A fairly large refactor for performance based mainly on this blog post about a fast implementation of geohash in Go, and the accompanying repository. I tried to include comments as best I could on some of the non-intuitive bit fiddling, but I think the blog post will do a better job explaining than I can.

In order to keep this compiling on stable, I didn't include any assembly, SIMD, or try to utilize a compiler intrinsic for the PDEP instruction. Even without going to those extremes, the bit manipulation and exploitation of IEEE 754 floating points was enough to give a pretty large performance boost. I was able to test on Windows, Ubuntu, and an M1 Mac and there were improvements on all platforms, including a speedup of decode from 78 ns/iter to 9 ns/iter on the M1 Mac.

Old:

New:

I also took the test cases (over 4000) from that repository and added them to a csv in the the tests folder here. The testing code uses serde and csv to deserialize each row into an instance of the TestCase struct, then run tests on it.

Other Changes:

• Added libm dependency. This is used for the ldexp function in error_with_precision and resulted in a pretty large speedup to decode
• Added a doc test for the neighbor function
• Added 2 new new error types InvalidHash for catching input hashes to decode that are too long and InvalidLength which catches inputs to the len parameter on encode greater than 12. Catching these allows us to avoid overflow checking in the base32 encoding and decoding when doing bit shifts
• Added some additional tests to check that errors are thrown appropriately.

👋 Hi @sunng87,

Following up on https://github.com/georust/meta/pull/21, I created a new geohash-publishers team with you as maintainer. Could you please update the crate owners on crates.io?

Specifically:

• [ ] update geohash crate owners on crates.io:

@sunng87 - you're the only "user owner" of that crate, so only you can edit that crate's owners:

cd geohash
# 💥 remove large publishing group
cargo owner --remove github:georust:core
# 🔐 add small publishing group
cargo owner --add github:georust:geohash-publishers

# Also, consider adding another contributor as a user-owner, that way if for some reason you become inaccessible
# we have someone else who can edit the owners. See https://github.com/rust-lang/crates.io/issues/2906
cargo owner --add <somebody>

# make sure everything looks good 👀 
cargo owner --list

And if there's anyone else you'd like to be able to publish the crate, feel free to add them to your publishing team, or as a user-owner. Consider that user-owners, as opposed to owners-via-team, can themselves edit the crate's owners, which could be desirable, or not, depending.

Let me know if you have any questions, or you can review https://github.com/georust/meta/pull/21.

Updates to the current version of geo, which now supports either f32 or f64 values. The hash looks like it expects f64 though, so I've bound the inputs and outputs to that.

CC: @sunng87

This issue was automatically generated. Feel free to close without ceremony if you do not agree with re-licensing or if it is not possible for other reasons. Respond to @cmr with any questions or concerns, or pop over to #rust-offtopic on IRC to discuss.

You're receiving this because someone (perhaps the project maintainer) published a crates.io package with the license as "MIT" xor "Apache-2.0" and the repository field pointing here.

TL;DR the Rust ecosystem is largely Apache-2.0. Being available under that license is good for interoperation. The MIT license as an add-on can be nice for GPLv2 projects to use your code.

Why?

The MIT license requires reproducing countless copies of the same copyright header with different names in the copyright field, for every MIT library in use. The Apache license does not have this drawback. However, this is not the primary motivation for me creating these issues. The Apache license also has protections from patent trolls and an explicit contribution licensing clause. However, the Apache license is incompatible with GPLv2. This is why Rust is dual-licensed as MIT/Apache (the "primary" license being Apache, MIT only for GPLv2 compat), and doing so would be wise for this project. This also makes this crate suitable for inclusion and unrestricted sharing in the Rust standard distribution and other projects using dual MIT/Apache, such as my personal ulterior motive, the Robigalia project.

Some ask, "Does this really apply to binary redistributions? Does MIT really require reproducing the whole thing?" I'm not a lawyer, and I can't give legal advice, but some Google Android apps include open source attributions using this interpretation. Others also agree with it. But, again, the copyright notice redistribution is not the primary motivation for the dual-licensing. It's stronger protections to licensees and better interoperation with the wider Rust ecosystem.

How?

To do this, get explicit approval from each contributor of copyrightable work (as not all contributions qualify for copyright, due to not being a "creative work", e.g. a typo fix) and then add the following to your README:

## License

Licensed under either of

 * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0)
 * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT)

at your option.

### Contribution

Unless you explicitly state otherwise, any contribution intentionally submitted
for inclusion in the work by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any
additional terms or conditions.

and in your license headers, if you have them, use the following boilerplate (based on that used in Rust):

// Copyright 2016 geohash.rs Developers
//
// Licensed under the Apache License, Version 2.0, <LICENSE-APACHE or
// http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
// http://opensource.org/licenses/MIT>, at your option. This file may not be
// copied, modified, or distributed except according to those terms.

It's commonly asked whether license headers are required. I'm not comfortable making an official recommendation either way, but the Apache license recommends it in their appendix on how to use the license.

Be sure to add the relevant LICENSE-{MIT,APACHE} files. You can copy these from the Rust repo for a plain-text version.

And don't forget to update the license metadata in your Cargo.toml to:

license = "MIT OR Apache-2.0"

I'll be going through projects which agree to be relicensed and have approval by the necessary contributors and doing this changes, so feel free to leave the heavy lifting to me!

Contributor checkoff

To agree to relicensing, comment with :

I license past and future contributions under the dual MIT/Apache-2.0 license, allowing licensees to chose either at their option.

Or, if you're a contributor, you can check the box in this repo next to your name. My scripts will pick this exact phrase up and check your checkbox, but I'll come through and manually review this issue later as well.

• [x] @sunng87
• [x] @frewsxcv

I noticed you still have a .travis.yml file. I believe free CI on travis-ci.org has been canceled since about June. Also, travis-ci recently had a nasty bug (and a not great response to said bug).

Almost all of the other georust projects are running on GH actions, would you be interested in migrating?

I could help with this if you have questions or lack the time.

Could it be possible to update the crate to 2018 edition before 0.9.0 release?

cargo fix --edition should do most of the work, only problems are the bench tests that use nightly and the command fails with the output:

error[E0554]: #![feature] may not be used on the stable release channel
 --> benches/base.rs:1:1
  |
1 | #![feature(test)]
  | ^^^^^^^^^^^^^^^^^

But if its run on the nightly channel it seems to work, and the code compiles on the stable as well :D

rustup run nightly cargo fix --edition

I noticed this when trying to get neighbors for the geohash 9g3m. The current implementation was duplicating some neighbors and returning an incorrect list.

It seems we accidentally had the x/y order flipped for the error values returned from the decode function. I suspect this escaped notice since the existing test cases had either very small errors or were at square-ish geohash levels where the x/y errors were not significantly different.

The library has it documented correctly as:

return value is `(<coordinate>, <longitude error>, <latitude error>)

so I think this change makes the implementation match the documentation (and match how we are using it).

I also checked these against the node implementation from a node js repl (although it's a little confusing because they print them in a different order):

> geohash.neighbors("ww8p1r4t8");
[ 'ww8p1r4tb',
  'ww8p1r4tc',
  'ww8p1r4t9',
  'ww8p1r4t3',
  'ww8p1r4t2',
  'ww8p1r4mr',
  'ww8p1r4mx',
  'ww8p1r4mz' ]

geohash.neighbors("9q60y60rhs");
[ '9q60y60rht',
  '9q60y60rhv',
  '9q60y60rhu',
  '9q60y60rhg',
  '9q60y60rhe',
  '9q60y60rh7',
  '9q60y60rhk',
  '9q60y60rhm' ]

geohash.neighbors("9g3m")
[ '9g3q', '9g3w', '9g3t', '9g3s', '9g3k', '9g3h', '9g3j', '9g3n' ]

This would make backwards compatibility nicer. I know precision wise it is pointless but we have some 16-length geohashes lying around and limiting to 12 is going to make migrating more difficult.