-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Conversation
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. |
Wondering if something like this would work? https://docs.microsoft.com/en-us/azure/devops/pipelines/process/container-phases?view=azure-devops |
@@ -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) |
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.
should np.intp
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.
nvm this is ok
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. |
Here's a working Dockerfile if it helps. This is just Py37, could extend to others simply using something like this: 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 |
we should drop 32 bit support (for 1.1) |
I think this and the other 32 bit fixup are fine to merge if it gets the wheel building moving along @TomAugspurger |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
What would be the best way to validate that this fixes the MacPython/pandas-wheels @TomAugspurger