Skip to content

Conversation

ShaharNaveh
Copy link
Contributor

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

Follow up to #32794 (comment)

@WillAyd
Copy link
Member

WillAyd commented Mar 21, 2020

Looks good. Could reduce the number of comments but not a blocker for me

@jbrockmendel were there any reasons to use shape instead of len with memory views? I think I remember you mentioning something about that.

@WillAyd WillAyd added the Clean label Mar 21, 2020
@jbrockmendel
Copy link
Member

were there any reasons to use shape instead of len with memory views? I think I remember you mentioning something about that.

Yah, for nearly the exact same reason its being done here: the .shape lookup happens in c-space while the __len__ call goes into pyspace. IIRC thats unintentional and expected to be changed in cython 0.30

LGTM; agreed that the comments can be less verbose, just one-liners TODO(cython0.30): use len instead of shape[0]

@WillAyd
Copy link
Member

WillAyd commented Mar 21, 2020

LGTM; agreed that the comments can be less verbose, just one-liners TODO(cython0.30): use len instead of shape[0]

@MomIsBestFriend can you do this?

@jreback jreback added this to the 1.1 milestone Mar 22, 2020
@jreback jreback merged commit 418b26a into pandas-dev:master Mar 22, 2020
@jreback
Copy link
Contributor

jreback commented Mar 22, 2020

thanks @MomIsBestFriend

should make an issue to change this once cython 0.30 is out

SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020
@ShaharNaveh ShaharNaveh deleted the CLN-avoid-casting branch March 23, 2020 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants