dask-cuda v23.4.0 #21
Conversation
…nda-forge-pinning 2023.04.12.20.43.24
regro-cf-autotick-bot
requested review from
jakirkham and
ocefpaf
as code owners
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
| - pandas >=1.0 | ||
| - pandas >=1.3,<1.6.0dev0 |
There was a problem hiding this comment.
We may need a repodata hot-fix to add this to old versions
There was a problem hiding this comment.
Is there a relevant issue to link to on why we're doing this? I see rapidsai/dask-cuda#1139 is where the max version constraint got introduced, but isn't clear to me if newer pandas versions were causing problems for older dask-cuda versions
There was a problem hiding this comment.
It looks like the upper bound on Pandas in RAPIDS has been around for a while. When upgrading to Pandas 1.5, the pinning was bumped to this upper bound ( rapidsai/integration#534 )
That said, I don't why Dask-CUDA needs the Pandas pin necessarily
Maybe one explanation is RAPIDS isn't compatible with Pandas 2.0 and needs an upper bound in multiple libraries?
cc @galipremsagar (in case you have more insight 🙂)
There was a problem hiding this comment.
We have been maintaining the same upper bounds across RAPIDS to make it easier to maintain and debug any conda issues. However I don't think we have tested dask-cuda with pandas-2.0 yet because we are still adding pandas-2.0 support in cudf, so only once we change the upper bound at cudf we should be able to test it in dask-cuda. I say we keep the pinnings consistent across RAPIDS for now and about applying the repodata hot-fix to older versions, we might not need it because cudf bring in that tight pinning. However, if you both feel it's trivial to fix it I'm okay with it aswell.
There was a problem hiding this comment.
Thanks for the context 🙂 I'm interested in if there are specific use cases for dask-cuda that don't involve cuDF where we might be able to get away with leaving this dependency unpinned (maybe @pentschev has some thoughts?), but if those cases don't exist / we aren't testing against them agree that pinning away from pandas 2.0 is best for now.
I can open a PR applying the repodata hot-fix and we can continue discussion there - not sure if we would get some pushback since my understanding is that the repodata patches are mostly to prevent explicit failures, but I know in the past we've done stuff like this for the dask-cuda package.
There was a problem hiding this comment.
There was a problem hiding this comment.
Arguably, for RAPIDS we should keep the pin that works with the rest of RAPIDS. However, this is the conda-forge package that gets used by the non-RAPIDS users from the community, for example, CuPy-based workflows. In here, I believe it makes sense that we follow the pins that the rest of Dask/Distributed use. Can we unpin pandas only here, while keeping the packages in rapidsai/rapidsai-nightly consistent with the remaining of RAPIDS?
There was a problem hiding this comment.
Think that would cause issues at least for this release, as my understanding is that this pinning is getting pulled from the pyproject.toml:
So my understanding is that even if we did pull pandas 2 with this package, it would throw an error due to the mismatch in expected/actual versions.
If we wanted to have separate pinnings for pandas between the conda-forge and RAPIDS package, I think we would need the pyproject pinning to ideally be the looser of the two pinnings?
There was a problem hiding this comment.
We discussed offline, and since we don't currently test against pandas 2.0 it's probably best for now to keep the pin. If some user requests this change then we can review this decision. In any case, the plan is to have support for pandas 2.0 around August, for RAPIDS 23.08.
|
Hi! This is the friendly conda-forge automerge bot! I considered the following status checks when analyzing this PR:
Thus the PR was passing and merged! Have a great day! |
|
@charlesbluca, could you please take a look at this one? Guessing this is related to |
|
IIUC the discussion above, it sounds like we are ok to merge this. Is that correct? |
|
Yup, and conda-forge/conda-forge-repodata-patches-feedstock#437 should address the follow up repodata hotfix |
Co-authored-by: Charles Blackmon-Luca <[email protected]>
|
Hi! This is the friendly conda-forge automerge bot! Commits were made to this PR after the |
|
Thanks all! 🙏 |
It is very likely that the current package version for this feedstock is out of date.
Checklist before merging this PR:
license_fileis packagedInformation about this PR:
@conda-forge-admin,please add bot automergein the title and merge the resulting PR. This command will add our bot automerge feature to your feedstock.bot-rerunlabel to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase@conda-forge-admin, please rerun botin a PR comment to have theconda-forge-adminadd it for you.Dependency Analysis
Please note that this analysis is highly experimental. The aim here is to make maintenance easier by inspecting the package's dependencies. Importantly this analysis does not support optional dependencies, please double check those before making changes. If you do not want hinting of this kind ever please add
bot: inspection: falseto yourconda-forge.yml. If you encounter issues with this feature please ping the bot teamconda-forge/bot.Analysis by grayskull shows a discrepancy between it and the the package's stated requirements in the meta.yaml.
Packages found by grayskull but not in the meta.yaml:
Packages found in the meta.yaml but not found by grayskull:
This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/4683270585, please use this URL for debugging.