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

CLN: Removed unused variable #32458

Closed

Conversation

ShaharNaveh
Copy link
Member

I can't see the reason for res_view to exist, the CI should tell me if I am wrong.

Also this is removing one warning when compiling pandas/_libs/internals.pyx

This is the warning that is removing:

pandas/_libs/internals.c:8952:36: warning: ‘__pyx_t_23’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 8952 |         __pyx_cur_scope->__pyx_t_9 = __pyx_t_23;


i = 0
for start, stop in slices:
for diff in range(start, stop):
res_view[i] = diff
Copy link
Member

Choose a reason for hiding this comment

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

this is equivalent to setting result[i] = diff. my guess is doing it like with the memoryview is intended for perf

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, the CI is not passing which is a clear sign that this is important.

Now all that I am wondering is how to get rid of that warnings during compilation.

Copy link
Member

Choose a reason for hiding this comment

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

Yea the warnings here are really the big blocker for #32163 . If you find a solution that would be huge!

@WillAyd
Copy link
Member

WillAyd commented Mar 6, 2020

@MomIsBestFriend we have a rather large queue atm so closing this one since I don't think would go forward with it anyway, but if you see a way to get the warnings out of this function would love a separate PR for that

@WillAyd WillAyd closed this Mar 6, 2020
@ShaharNaveh ShaharNaveh deleted the CLN-res_view-internals branch March 14, 2020 14:07
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