Skip to content
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

Switch from pin_compatible to explicit exclusion of nightly versions #193

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

charlesbluca
Copy link
Member

Turns out we need a tighter pinning than what (from my understanding) pin_compatible is able to generate - for example, generated pinning >=2022.8.1,<2022.8.2.0a0 would allow us to install dask 2022.8.1 with 2022.8.2a packages.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

Comment on lines +21 to +22
- dask-core {{ version }}.*,!={{ version }}a.*
- distributed {{ version }}.*,!={{ version }}a.*
Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding here is that nightly versions get parsed by conda as something like 2022.8.1a.220805, so the wildcard exclusion here should always be valid, I just think for some reason conda isn't a fan of exclusion wildcards without the .; either way, I've opened conda/conda#11744 to document this behavior.

@jakirkham
Copy link
Member

Appreciate the persistence in trying to get things working correctly here Charles 🙏

Am having trouble following why 2022.8.2a would be different from 2022.8.2.0a. AFAICT those should be the same version. It is already common for versions to exclude additional values that may be implied .0s. This sounds like a bug we should raise. Would check to see if this occurs with conda or mamba (or both) and raise appropriately.

In any event it feels like we are going to a lot of work to avoid using strict channel priority, which is what we should ideally be using here. It may be worth taking a deeper look at how we can move to strict channel priority to avoid these kinds of issues.

Potentially another kludge (if we can't get strict channel priority to work) is just to prepend the channel we want on to the relevant packages. Though am really hoping we can find a cleaner solution (like strict channel priority).

@charlesbluca
Copy link
Member Author

Am having trouble following why 2022.8.2a would be different from 2022.8.2.0a

Based on the version ordering scheme outlined here, I would think this is expected behavior? IIUC, the two versions would parse out to something like

# adding 0s for missing correspondents
2022.8.2a   => [[0], [2022], [8], [2, 'a'], [0, 0]]
2022.8.2.0a => [[0], [2022], [8], [2, 0], [0, 'a']]

And then [2, 'a'] < [2, 0] would decide that the latter version is greater than the former; not sure if I'm misunderstanding the documentation in terms of how 0s would get added to handle missing correspondents.

I'm also in favor of figuring out how to unblock strict channel priority so that the solution doesn't have to be a Dask specific thing, my main reason for pushing on this as a short-term fix is due to an unclear time table on how long it would take to sort out strict channel priority for RAPIDS-Dask. Would you be okay with merging something like this in for now, with a medium-term goal of hashing out strict channel priority so that we can remove these constraints down the line?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants