Skip to content

Allow version 0.19 of DataStructures #101

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

Closed
wants to merge 1 commit into from

Conversation

andreasnoack
Copy link
Collaborator

@andreasnoack andreasnoack commented Aug 3, 2025

Might be good to set up CompatHelper to simplify this although there is only a single dependency.

Might also be better to test on pre instead of nightly.

@DilumAluthge
Copy link
Member

Looks like CI isn't using DataStructures 0.19.

Example CI log: https://github.com/JuliaCollections/SortingAlgorithms.jl/actions/runs/16706927424/job/47286361683?pr=101

⌅ [864edb3b] DataStructures v0.18.22

@andreasnoack
Copy link
Collaborator Author

Okay. Then I'll set up CompatHelper. That will make it easier to resolve.

@andreasnoack andreasnoack deleted the patch-1 branch August 3, 2025 18:22
@LilithHafner
Copy link
Member

CompatHelper won't fix this. The problem is that we load StatsBase in testing and StatsBase does not declare compatibility with .19. This is a chicken and egg problem with JuliaStats/StatsBase.jl#959. The solution is to trick Julia & Pkg into disregarding StatsBase (and other test deps') declared incompatibility with DataStructures .19. @DilumAluthge, this seems like something that would come up fairly frequently. Is there a built in mechanism to do this?

@DilumAluthge
Copy link
Member

I think we have two different problems here.

Problem 1: Detecting this situation

In this PR, CI looks green, but didn't use the desired version of DataStructures.

In julia-runtest, we have special logic to detect CompatHelper PRs. If a CompatHelper PR is detected, then julia-runtest will force the latest version of dependencies (and will throw an error if that's not possible). So the underlying incompatibility still exists, but at least we get a visible red X in CI, and thus it's easier for us to notice that this problem exists.

This logic (in julia-runtest) only applies to CompatHelper PRs. It doesn't apply to human PRs, which is why julia-runtest doesn't run that logic in this PR, and thus why we have a deceptive green CI checkmark on this PR.

Problem 2: Chicken/egg problem between this package and StatsBase

I don't know of a built-in mechanism for solving this problem. It may be worth opening an issue on Pkg.jl.

One manual workaround for us to use here might be:

  1. Modify the CI script in this repo to manually Pkg.develop() the CompatHelper branch of StatsBase (the branch for CompatHelper: bump compat for DataStructures to 0.19, (keep existing compat) JuliaStats/StatsBase.jl#959).
  2. Get CI passing here with the latest 0.19 of DataStructures, and merge PR here.
  3. Tag new release here.
  4. Wait for new release of this report to be merged into General and propagated to Pkg servers
  5. Rebase CompatHelper: bump compat for DataStructures to 0.19, (keep existing compat) JuliaStats/StatsBase.jl#959 (or just close/reopen the PR, if it is already up-to-date with master).
  6. Now CI on CompatHelper: bump compat for DataStructures to 0.19, (keep existing compat) JuliaStats/StatsBase.jl#959 should be using 0.19 of DataStructures.
  7. Merge CompatHelper: bump compat for DataStructures to 0.19, (keep existing compat) JuliaStats/StatsBase.jl#959
  8. Tag new release of StatsBase
  9. Wait for new release of this report to be merged into General and propagated to Pkg servers
  10. REVERT step 1 (the Pkg.develop() in this repo).

@LilithHafner
Copy link
Member

CompatHelper is also good because that will give watchers a notification whenever a breaking release of DataStructures is cut.

@LilithHafner
Copy link
Member

I'd squeeze step 10 into step 2 between "Get CI passing here with the latest 0.19 of DataStructures" and "merge PR here."

@devmotion
Copy link
Contributor

Can we just remove StatsBase from the list of test dependencies? Circular dependencies are bad in general IMO.

@LilithHafner
Copy link
Member

LilithHafner commented Aug 4, 2025

In this case, yes. But in general, that's not always viable. It's quite reasonable to want to use tooling packages when testing and it's also reasonable for tooling to have deps.

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.

4 participants