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

Add faces, distance and miscellaneous functions #46

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

retbrown
Copy link
Contributor

@retbrown retbrown commented Mar 3, 2021

Add functions to cover all functions in the current version of H3 except the two experimental prefixed functions (experimentalH3ToLocalIj and experimentalLocalIjToH3)

At a minimum it covers the functions mentioned in #29 and #44

@coveralls
Copy link

coveralls commented Mar 3, 2021

Pull Request Test Coverage Report for Build 617809629

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 611964428: 0.0%
Covered Lines: 278
Relevant Lines: 278

💛 - Coveralls

@retbrown retbrown force-pushed the add-missing-functions-v2 branch from 3d9edd6 to af57f7b Compare March 3, 2021 14:16
@retbrown retbrown changed the title Add faces and distance between functions Add faces, distance and miscellaneous functions Mar 3, 2021
@retbrown retbrown force-pushed the add-missing-functions-v2 branch from af57f7b to f0831fc Compare March 3, 2021 14:17
@phanirithvij
Copy link

I think you can remove #45 from this PR's description.
I'd also like to know when this will be reviewed/merged?

@phanirithvij
Copy link

phanirithvij commented Mar 3, 2021

Also, this code is giving me all 0s when using your fork, all the three ExactEdgeLength.. methods give 0

package main

import (
	"fmt"

	"github.com/uber/h3-go/v3"
)

func main() {
	pt := h3.GeoCoord{
		Latitude:  38.8935128,
		Longitude: -77.1546608,
	}
	for i := 0; i < 16; i++ {
		h := h3.FromGeo(pt, i)
		fmt.Println(h3.ExactEdgeLengthRads(h), h3.ExactEdgeLengthKm(h), h3.ExactEdgeLengthM(h))
	}
}

I'm running it with a go.mod setup like this

module example.org/server

go 1.16

replace github.com/uber/h3-go/v3 => github.com/retbrown/h3-go/v3 v3.0.3-0.20210303141742-f0831fcf37f8

require github.com/uber/h3-go/v3 v3.7.0

Should I open a new issue here, after this gets merged?

I didn't try out the C implementation because I'm assuming it's hard to set it up on windows. I will open an issue over at the h3 repo after trying it out on my Linux machine

@retbrown
Copy link
Contributor Author

retbrown commented Mar 3, 2021

The ExactEdgeLength* methods all require an edge index rather than a cell index. So rather than just passing the h3 index you would need to pass the index for one of the edges. I have modified your code above to do that below. You would need an additional call to ToUnidirectionalEdges and in the example i have just used the first entry in the returned slice to calculate the length from.

package main

import (
	"fmt"

	"github.com/uber/h3-go/v3"
)

func main() {
	pt := h3.GeoCoord{
		Latitude:  38.8935128,
		Longitude: -77.1546608,
	}
	for i := 0; i < 16; i++ {
		h := h3.FromGeo(pt, i)
		fmt.Println(h3.ExactEdgeLengthRads(h3.ToUnidirectionalEdges(h)[0]), h3.ExactEdgeLengthKm(h3.ToUnidirectionalEdges(h)[0]), h3.ExactEdgeLengthM(h3.ToUnidirectionalEdges(h)[0]))
	}
}

Outputs

$ go run main.go
0.1907659917703397 1215.371503443869 1.215371503443869e+06
0.07426317049993154 473.1311925328369 473131.1925328369
0.02893347635775318 184.33538564418043 184335.38564418044
0.010826730357448312 68.97717685317124 68977.17685317124
0.004167017952265829 26.548101296901795 26548.101296901794
0.0015478829104757237 9.861573137861825 9861.573137861826
0.000595086120136727 3.7912979446560024 3791.2979446560025
0.00022113288344402618 1.408839188359099 1408.839188359099
8.503464557269192e-05 0.5417563375704776 541.7563375704776
3.159208403445066e-05 0.20127339424366508 201.27339424366508
1.2148338271555708e-05 0.07739715036430815 77.39715036430815
4.513240645524859e-06 0.02875388856185201 28.753888561852012
1.7354751499254653e-06 0.011056724642480706 11.056724642480706
6.447497860535891e-07 0.004107705516843067 4.107705516843067
2.47925020279934e-07 0.0015795320845328183 1.5795320845328182
9.210711662312412e-08 0.0005868151014196192 0.5868151014196192

Copy link
Collaborator

@jogly jogly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outstanding!

@phanirithvij
Copy link

phanirithvij commented Mar 11, 2021

@retbrown can this be merged?

@retbrown
Copy link
Contributor Author

Hi @phanirithvij, Sadly I don't have permissions on the repo to merge any code

@jogly jogly merged commit cd94145 into uber:master Mar 15, 2021
@retbrown retbrown deleted the add-missing-functions-v2 branch March 16, 2021 15:37
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

Successfully merging this pull request may close these issues.

4 participants