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 vertex mode functions #138

Merged
merged 5 commits into from
Jul 22, 2022
Merged

Conversation

isaacbrodsky
Copy link
Collaborator

@isaacbrodsky isaacbrodsky commented Jun 23, 2022

Adds vertex mode functions and tests ported from H3-Java. Depends on #136.

@coveralls
Copy link

coveralls commented Jun 23, 2022

Pull Request Test Coverage Report for Build 2709180137

  • 21 of 21 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 2666354219: 0.0%
Covered Lines: 531
Relevant Lines: 531

💛 - Coveralls

@nrabinowitz
Copy link
Collaborator

This looks good (once rebased), but I can't help but feel that these functions should come with some big caveats about usage, and might be hard for an end user to take advantage of, which is part of the reason I hadn't added them to the bindings :/.

- Updated the core library to `v4.0.0-rc2`. This update renames the majority of the H3 functions. You can see a [list of changed function names](https://h3geo.org/docs/next/library/migration-3.x/functions) in the core library documentation. For the most part, upgrading to v4 for Javascript consumers should be a straightforward search & replace between the old names and the new.
- Added more cases in which JS errors may be thrown. In H3 v3, many functions would fail silently with invalid input, returning `null` or similar signal values. In H3 v4, we will throw descriptive errors for most instances of bad input.
- Updated the core library to `v4.0.0-rc2`. This update renames the majority of the H3 functions. You can see a [list of changed function names](https://h3geo.org/docs/next/library/migration-3.x/functions) in the core library documentation. For the most part, upgrading to v4 for Javascript consumers should be a straightforward search & replace between the old names and the new. (#139)
- Added more cases in which JS errors may be thrown. In H3 v3, many functions would fail silently with invalid input, returning `null` or similar signal values. In H3 v4, we will throw descriptive errors for most instances of bad input. (#139)
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙏

@@ -8,7 +8,7 @@
[![Coverage Status](https://coveralls.io/repos/github/uber/h3-js/badge.svg?branch=master)](https://coveralls.io/github/uber/h3-js?branch=master)
[![License](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](LICENSE)
[![npm version](https://badge.fury.io/js/h3-js.svg)](https://badge.fury.io/js/h3-js)
[![H3 Version](https://img.shields.io/badge/h3_api-v4.0.0-rc2-blue.svg)](https://github.com/uber/h3/releases/tag/v4.0.0-rc2)
[![H3 Version](https://img.shields.io/static/v1?label=h3%20api&message=v4.0.0-rc2&color=blue)](https://github.com/uber/h3/releases/tag/v4.0.0-rc2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catch, thanks

@isaacbrodsky isaacbrodsky merged commit d052e6b into uber:master Jul 22, 2022
@isaacbrodsky isaacbrodsky deleted the dev-v4-vertex branch July 22, 2022 00:51
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.

None yet

3 participants