Skip to content

[REVIEW] Update minimum pandas and numpy pinnings - #12887

Merged
rapids-bot[bot] merged 10 commits into
NVIDIA:branch-23.04from
galipremsagar:pandas_min_130
Mar 10, 2023
Merged

[REVIEW] Update minimum pandas and numpy pinnings#12887
rapids-bot[bot] merged 10 commits into
NVIDIA:branch-23.04from
galipremsagar:pandas_min_130

Conversation

@galipremsagar

@galipremsagar galipremsagar commented Mar 6, 2023 •
edited by bdice
Loading

Copy link
Copy Markdown
Contributor

Description

This PR:

  • Increments the minimum pinning for pandas version from 1.0 to 1.3.
  • Sets a minimum pinning for numpy as >=1.21
  • Fixes arm conda environment creation by removing pandoc version constraint.

Resolves #12785.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@galipremsagar galipremsagar added 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. 4 - Needs cuDF (Python) Reviewer improvement Improvement / enhancement to an existing function breaking Breaking change labels Mar 6, 2023
@galipremsagar
galipremsagar requested review from a team as code owners March 6, 2023 19:03
@galipremsagar galipremsagar self-assigned this Mar 6, 2023
@github-actions github-actions Bot added the conda label Mar 6, 2023
@bdice

bdice commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

Can we also bump numpy (currently it's unpinned) to align with dask and resolve this issue? #12785

@galipremsagar galipremsagar changed the title [REVIEW] Bump minimum pandas version from 1.0 to 1.3 [REVIEW] Update minimum pandas and numpy pinnings Mar 6, 2023
@galipremsagar

galipremsagar commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

Can we also bump numpy (currently it's unpinned) to align with dask and resolve this issue? #12785

Done 👍

@galipremsagar galipremsagar mentioned this pull request Mar 6, 2023
Merged
bdice
bdice reviewed Mar 6, 2023
View reviewed changes
Comment thread python/cudf/cudf/tests/test_rolling.py Outdated
Comment thread python/cudf/cudf/tests/test_reshape.py Outdated
Comment thread python/cudf/cudf/core/_compat.py Outdated
wence-
wence- reviewed Mar 7, 2023
View reviewed changes

@wence- wence- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor issues.

Comment thread python/cudf/cudf/tests/test_reshape.py Outdated
Comment thread python/cudf/cudf/tests/test_rolling.py Outdated
Comment thread python/cudf/cudf/core/_compat.py Outdated
@galipremsagar
galipremsagar requested review from bdice and wence- March 7, 2023 19:08
bdice
bdice reviewed Mar 7, 2023
View reviewed changes
Comment thread conda/recipes/cudf/meta.yaml
- cupy >=9.5.0,<12.0.0a0
- numba >=0.56.2
- numpy
- numpy >=1.21

@bdice bdice Mar 7, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current versions of numba require numpy<1.24 (until numba 0.57 is released with this fix). We're getting this pinning from the upstream feedstock but this pinning doesn't work if the user installs numba another way.

@jayrodge ran into this locally when installing RAPIDS 23.02 on WSL, perhaps with a numba from some source other than conda-forge.

Should we actively prevent that problem by pinning like this until numba 0.57 is released?

Suggested change
- numpy >=1.21
- numpy >=1.21,<1.24

@galipremsagar galipremsagar Mar 9, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even if we pin numpy in our feedstock, if the user install numba or as a matter of fact numpy in another we wouldn't be able to prevent these kinds of breakages no? I'm only hesitant on having an upper bound on numpy because we don't use it's internal APIs and this will have a maintenance overhead of tracking numpy releases and unpinning them manually. Whereas leaving it unpinned on the upper bound could help us detect breakages in our CI and have it fixed ASAP.

@bdice bdice Mar 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand the hesitation. I think this is a numba problem, and not something we should change in our dependencies. I just wanted to pose the idea. Let’s leave it alone.

@wence-

wence- commented Mar 10, 2023

Copy link
Copy Markdown
Contributor

I think this is just waiting to resolve the numpy upper bound pin for numba < 0.57, but is otherwise good to go?

@bdice

bdice commented Mar 10, 2023

Copy link
Copy Markdown
Contributor

I think this is just waiting to resolve the numpy upper bound pin for numba < 0.57, but is otherwise good to go?

Let’s not block on that. I think we may take no action on that upstream problem.

wence-
wence- approved these changes Mar 10, 2023
View reviewed changes

@wence- wence- left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@galipremsagar galipremsagar mentioned this pull request Mar 10, 2023
Merged
@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Mar 10, 2023
rapids-bot Bot pushed a commit to rapidsai/dask-cuda that referenced this pull request Mar 10, 2023
@galipremsagar
Update minimum pandas and numpy pinnings (#1139)
cdd4c0b 
This PR updates `pandas` & `numpy` pinnings to be in-sync with `cudf`: NVIDIA/cudf#12887

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)
  - Ray Douglass (https://github.com/raydouglass)

URL: #1139
@galipremsagar

galipremsagar commented Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit e4557cb into NVIDIA:branch-23.04 Mar 10, 2023
@vyasr vyasr mentioned this pull request Mar 14, 2023
Merged
3 tasks done
@vyasr vyasr added dask Dask issue and removed dask-cudf labels Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wence- wence- wence- approved these changes
@bdice bdice bdice approved these changes
@shwina shwina Awaiting requested review from shwina shwina was automatically assigned from rapidsai/cudf-python-codeowners
@brandon-b-miller brandon-b-miller Awaiting requested review from brandon-b-miller brandon-b-miller was automatically assigned from rapidsai/cudf-python-codeowners
+1 more reviewer
@raydouglass raydouglass raydouglass approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@galipremsagar galipremsagar

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge breaking Breaking change dask Dask issue improvement Improvement / enhancement to an existing function Python Affects Python cuDF API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bump minimum pandas/numpy versions

5 participants

@galipremsagar @bdice @wence- @raydouglass @vyasr