-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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
BLD: remove MSVC workaround #45008
BLD: remove MSVC workaround #45008
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this bump up min versions of any windows things?
im not aware of anywhere that we specify those |
We're still on VS 2017 on the wheel repo. This needs to wait for MacPython/pandas-wheels#160. |
Thanks @lithomas1. Is there an ETA on that? i.e. should this be closed or just given the "Blocked" label? |
That patch is scheduled to land for the 1.4rc, but this'll probably have to land in 2.0 because we'll have to wait until after 1.4.0 is released to make sure that no issues are reported that will cause it to be rolled back(last time we upgraded to Visual Studio 2019, we had missing DLL issues). |
changing milestone to 1.5 |
@lithomas1 looks like MacPython/pandas-wheels#160 has been merged, does that mean we're good to go here? |
I think so. |
ok let's give this a go, thanks @lithomas1 might be worth updating install.rst with the min required MSVC version? |
- Pandas not only removed upstream a fix for MSVC but also removed what allowed us to have working std::signbit, see: pandas-dev/pandas#45008 - Revert that commit partially so that we fix the build error: pandas/_libs/window/aggregations.cpp:4951:18: error: 'signbit' was not declared in this scope
isnan
once MSVC bug is fixed #23209Shot in the dark to see if the upstream problem behind #23209 is resolved.