Skip to content

BLD: Remove windows shim #46437

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 23, 2022
Merged

BLD: Remove windows shim #46437

merged 1 commit into from
Apr 23, 2022

Conversation

jbrockmendel
Copy link
Member

Just to see if this breaks the world.

@jreback jreback added the Build Library building on various platforms label Mar 20, 2022
@jbrockmendel jbrockmendel mentioned this pull request Mar 21, 2022
Open
lithomas1
lithomas1 approved these changes Mar 24, 2022
View reviewed changes
Copy link
Member

@lithomas1 lithomas1 left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe someone more familiar with the C code should review though.

pandas/_libs/src/headers/stdint.h
#ifndef _PANDAS_STDINT_H_
#define _PANDAS_STDINT_H_

#if defined(_MSC_VER) && (_MSC_VER < 1900)
Copy link
Member

Choose a reason for hiding this comment

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

I guess the shim was only used for < VS 2015(https://dev.to/yumetodo/list-of-mscver-and-mscfullver-8nd). We should be good here as our min. Visual Studio requirement should be VS2019.

Copy link
Member

Choose a reason for hiding this comment

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

More of out curiosity than anything else, where do we specify the minimum version for VS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely no idea I opened cython/cython#2570 years ago in the hope this would become their problem

Copy link
Member

Choose a reason for hiding this comment

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

More of out curiosity than anything else, where do we specify the minimum version for VS?

There's a mention of VS2019 in our development environment docs

https://pandas.pydata.org/pandas-docs/stable/development/contributing_environment.html#installing-a-c-compiler

@lithomas1 lithomas1 added this to the 1.5 milestone Mar 24, 2022
mroeschke
mroeschke approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

Just in case, I think it would be good to include a whatsnew note that a customs header for int types for VS 2019 compat has been removed.

@jbrockmendel
Copy link
Member Author

Just in case, I think it would be good to include a whatsnew note that a customs header for int types for VS 2019 compat has been removed.

I don't see how it's user-facing, but OK. Suggested wording/location?

@mroeschke mroeschke merged commit 4caa297 into pandas-dev:main Apr 23, 2022
@mroeschke
Copy link
Member

Okay on second thought, this should be okay. Thanks @jbrockmendel

@rhshadrach
Copy link
Member

rhshadrach commented Apr 23, 2022 •
edited
Loading

Looks like this also fixed the py38_32bit build.

Nevermind, the run was from before this build started failing.

@jbrockmendel jbrockmendel deleted the bld-shim branch April 24, 2022 02:43
yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
@jbrockmendel @yehoshuadimarsky
Remove windows shim (pandas-dev#46437)
5dc02b3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhshadrach rhshadrach rhshadrach left review comments

@mroeschke mroeschke mroeschke approved these changes

@lithomas1 lithomas1 lithomas1 approved these changes

Assignees
No one assigned
Labels
Build Library building on various platforms
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.
5 participants
@jbrockmendel @mroeschke @rhshadrach @lithomas1 @jreback