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

Support str-like values for str_to_int #408

Merged
merged 2 commits into from
Oct 5, 2024

Conversation

jongbinjung
Copy link
Collaborator

@jongbinjung jongbinjung commented Oct 5, 2024

Since numpy>=2 (?), str values are not native str but numpy.str_.

Without the explicit conversion to str, the test fails with:

================================================== FAILURES ===================================================
_______________________________________________ test_numpy_str ________________________________________________

    def test_numpy_str():
        expected = [
            617700169957507071,
            617700169957769215,
            617700169958031359,
            617700169958293503,
            617700169961177087,
            617700169964847103,
            617700169965109247,
        ]
        cells = np.array([h3.int_to_str(h) for h in expected])
>       got = [h3.str_to_int(c) for c in cells]

tests/test_apis/test_numpy_int.py:50:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/test_apis/test_numpy_int.py:50: in <listcomp>
    got = [h3.str_to_int(c) for c in cells]
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

h = np.str_('89283082803ffff')

    def str_to_int(h):
        """
        Converts a hexadecimal string to an H3 64-bit integer index.

        Parameters
        ----------
        h : str
            Hexadecimal string like ``'89754e64993ffff'``

        Returns
        -------
        int
            Unsigned 64-bit integer
        """
        # return _cy.str_to_int(str(h))
>       return _cy.str_to_int(h)
E       TypeError: Argument 'h' has incorrect type (expected str, got numpy.str_)

env/lib/python3.11/site-packages/h3/api/numpy_int/__init__.py:57: TypeError

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2024

CLA assistant check
All committers have signed the CLA.

@ajfriend
Copy link
Contributor

ajfriend commented Oct 5, 2024

Nice! Can we bump it back to the failing state just to see the issue in CI? I might just need to make sure it is allowed to run.

@jongbinjung
Copy link
Collaborator Author

@ajfriend Force-pushed to the commit at failing state. Waiting for approval to run tests.

@ajfriend
Copy link
Contributor

ajfriend commented Oct 5, 2024

Hmmm. Not clear why coverage isn't working, especially since it seems to be working on #409

@jongbinjung
Copy link
Collaborator Author

Hmmm. Not clear why coverage isn't working, especially since it seems to be working on #409

Seems like a known issue with codecov action and PRs from a fork ...

@ajfriend ajfriend mentioned this pull request Oct 5, 2024
@ajfriend
Copy link
Contributor

ajfriend commented Oct 5, 2024

OK. I think this is good to land, but it sounds like we need to figure out an alternative to codecov going forward: #393

@ajfriend ajfriend merged commit 98bf22a into uber:master Oct 5, 2024
69 of 71 checks passed
ajfriend pushed a commit that referenced this pull request Oct 5, 2024
* Add failing test

* Explicitly convert to str before calling _cy.str_to_int
ajfriend added a commit that referenced this pull request Oct 5, 2024
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