Skip to content

BUG: Fix failing MacPython 32bit wheels for groupby rolling #34423

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

Merged
merged 1 commit into from
May 29, 2020

Conversation

mroeschke
Copy link
Member

What would be the best way to validate that this fixes the MacPython/pandas-wheels @TomAugspurger

@mroeschke mroeschke added 32bit 32-bit systems Bug Window rolling, ewma, expanding labels May 28, 2020
@mroeschke mroeschke changed the title BUG: Fix failing MacPython 32bit wheels for rolling groupby BUG: Fix failing MacPython 32bit wheels for groupby rolling May 28, 2020
@TomAugspurger
Copy link
Contributor

If you really wanted to, you could build a docker image with these changes.

FROM quay.io/pypa/manylinux1_i686:latest  # verify this is the right image.

RUN git clone --depth=1 https://github.com/mroeschke/pandas/mac32_rolling \
 && cd pandas \
 && /opt/python/cp37-cp37/bin/python -m pip install numpy cython python-dateutil pytz
WORKDIR pandas

RUN /opt/python/cp38-cp38/bin/python setup.py build_ext -i

We really need to get this hooked up to our CI here, but it isn't 100% straightforward.

@TomAugspurger TomAugspurger added this to the 1.1 milestone May 28, 2020
@WillAyd
Copy link
Member

WillAyd commented May 28, 2020

@@ -218,16 +218,18 @@ def get_window_bounds(
start, end = indexer.get_window_bounds(
len(indicies), min_periods, center, closed
)
start = start.astype(np.int64)
Copy link
Contributor

Choose a reason for hiding this comment

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

should np.intp

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm this is ok

@TomAugspurger
Copy link
Contributor

Wondering if something like this would work?

Yeah that's 100% what we need. Just integrating it into the current layout isn't straightforward.

There's also the fact that Anaconda stopped building 32-bit linux package, making this somewhat hard to test.

@WillAyd
Copy link
Member

WillAyd commented May 28, 2020

Here's a working Dockerfile if it helps. This is just Py37, could extend to others simply using something like this:

https://github.com/pypa/python-manylinux-demo/blob/84dc72039a0c85c1c960e024f024fd3d37387634/travis/build-wheels.sh#L18

FROM quay.io/pypa/manylinux1_i686:latest

RUN git clone -b mac32_rolling https://github.com/mroeschke/pandas.git \
 && cd pandas \
 && /opt/python/cp37-cp37m/bin/python -m pip install numpy cython python-dateutil pytz
WORKDIR pandas

RUN /opt/python/cp37-cp37m/bin/python setup.py build_ext -i -j4

FWIW still fails to build but I think errors are just what @TomAugspurger is fixing in #34416

For this change in particular (being python-level only) we probably need an extra step to actually execute the tests if we wanted to verify quickly that way

@jreback
Copy link
Contributor

jreback commented May 28, 2020

we should drop 32 bit support (for 1.1)

@WillAyd
Copy link
Member

WillAyd commented May 29, 2020

I think this and the other 32 bit fixup are fine to merge if it gets the wheel building moving along @TomAugspurger

@TomAugspurger TomAugspurger merged commit cb244ed into pandas-dev:master May 29, 2020
@mroeschke mroeschke deleted the mac32_rolling branch May 29, 2020 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
32bit 32-bit systems Bug Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

32-bit compat issues in MacPython/pandas-wheels
4 participants