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

ENH: Add annotations for scipy.spatial.cKDTree #14024

Merged
merged 3 commits into from Jun 3, 2021
Merged

Conversation

BvB93
Copy link
Contributor

@BvB93 BvB93 commented May 11, 2021

Reference issue

n.a.

What does this implement/fix?

This PR adds annotations to the cKDTree and cKDTreeNode classes for the purpose of static type checking.

Additional information

  • There were two methods where different output types are returned for 1D vs >= 2D array-likes (see below).
    As we cannot describe array shapes/dimensionality as of yet, the output type is set to Any here.
  • There are 2 checks for 0D array-likes, the latter currently excluding 0D arrays as (again) we cannot type
    array shapes/dimensionality yet. This does mean that there is some small room for a false positive here,
    though these are very much edge cases in my opinion.

@BvB93
Copy link
Contributor Author

BvB93 commented May 11, 2021

Tests failures seem to be due to #14014 thus far.

Copy link
Member

@tupui tupui left a comment

Choose a reason for hiding this comment

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

IMO we should follow what we did in #13833. Have inline typing and also for int,float use the things from scipy._lib._util that this PR will introduce.

@tupui tupui added scipy.spatial static typing Items related to static typing labels May 11, 2021
@BvB93
Copy link
Contributor Author

BvB93 commented May 11, 2021

As far as I'm aware inline type hinting is not possible here, as the source is based on cython .pyx files.
Should be doable for KDTree though, but that's beyond the scope of this PR.

@tupui
Copy link
Member

tupui commented May 11, 2021

As far as I'm aware inline type hinting is not possible here, as the source is based on cython .pyx files.
Should be doable for KDTree though, but that's beyond the scope of this PR.

Ah right, I did see this was for cython files.

scipy/spatial/ckdtree.pyi Outdated Show resolved Hide resolved
scipy/spatial/ckdtree.pyi Show resolved Hide resolved
scipy/spatial/ckdtree.pyi Show resolved Hide resolved
Bas van Beek and others added 3 commits May 15, 2021 16:41
@tupui
Copy link
Member

tupui commented Jun 3, 2021

As discussed during the meetup. This is ready so I am merging. Thanks @BvB93!

@tupui tupui merged commit 2c72ae2 into scipy:master Jun 3, 2021
@tylerjereddy tylerjereddy added this to the 1.8.0 milestone Jun 4, 2021
@BvB93 BvB93 deleted the kdtree branch June 22, 2021 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scipy.spatial static typing Items related to static typing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants