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

Fix error checking on hex2int #127

Merged
merged 1 commit into from
May 31, 2020
Merged

Fix error checking on hex2int #127

merged 1 commit into from
May 31, 2020

Conversation

ajfriend
Copy link
Contributor

Change from

cpdef H3int hex2int(H3str h) except 0

to

cpdef H3int hex2int(H3str h) except? 0

The previous version considered any return of 0 an error, which led to confusing error messages for the user. This function simply converts; check for a valid cell happens after conversion.

Changing except 0 to except? 0 makes it so that Python only checks if there is an error when 0 is returned. We need some C-compatible sentinel value to flag actual/potential errors for pure C/Cython functions.

Old error message:
Screen Shot 2020-05-30 at 11 46 54 AM

New error message:
Screen Shot 2020-05-30 at 11 44 51 AM

This PR also adds a test to verify that we get a helpful error message in this example.

@codecov
Copy link

codecov bot commented May 30, 2020

Codecov Report

Merging #127 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #127   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          181       181           
=========================================
  Hits           181       181           

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 944d917...8a6bdd8. Read the comment docs.

Copy link
Collaborator

@isaacbrodsky isaacbrodsky left a comment

Choose a reason for hiding this comment

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

I wonder how other Python libraries handle ensuring the user doesn't pass in incorrect types. I seem to recall Pandas giving me an error about types that was more specific, but I don't recall exactly what function call for that right now.

@ajfriend
Copy link
Contributor Author

I'll land this for now, but more than happy to revisit if we have ideas around better error reporting.

Pandas might have it a little easier because it does have typed arrays for columns, in the form of numpy arrays, but I'm not sure if that's the error message you were thinking of.

@ajfriend ajfriend merged commit 1b78cce into master May 31, 2020
@ajfriend ajfriend deleted the hex2int branch May 31, 2020 20:57
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.

3 participants