Bk/fix faster orderings

[bkamins]

Fix #3358
and prepares 1.6.1 patch release.

[bkamins]

CC @alonsoC1s

[davidanthoff]

I have to admit I'm generally not a fan of this kind of dependency handling. Why not just do this the normal way, add DataStructures as a dependency and put a compat bound into the Project.toml?

This solution here seems very brittle, all it needs to break this again is for DataStructures to make some incompatible change to the implementation of FasterForward. If DataStructures then tags a breaking release (correctly), then there will be a DataFrames version in the general registry that the package resolver assumes will work with that new version of DataStructures, but in fact it doesn't. You could then of course do something similar again to this PR for a new release of DataFrames, but in my mind having these old versions in the general registry that declare incorrect version compatibilities is really not good.

Maybe the new weak deps stuff could be used to handle this properly? I haven't looked into that much, though.

[bkamins]

Why not just do this the normal way, add DataStructures as a dependency and put a compat bound into the Project.toml?

Exactly for the reason you have mentioned in your original post - to avoid Julia package manager to install DataFrames.jl 1.6.0 incorrectly with old DataStructures.jl.

After this PR another PR for DataFrames.jl 1.6.2 (or 1.7.0 whichever comes first) with adding a bound for DataStructures.jl should be added. Then we have 1.6.1 working with old DataStructures.jl and some newer version (in a few months at most) that handles only new DataStructures.jl.

Now what you describe indeed can happen, but if this happens then DataStructures.jl should bump its version, and then SortingAlgorithms.jl should bump its version to 2.0, so the proposed DataFrames.jl 1.6.1 release would not get installed.

---

I agree what I propose is not fully clean, but I hope it will be robust enough.

[davidanthoff]

if this happens then DataStructures.jl should bump its version, and then SortingAlgorithms.jl should bump its version to 2.0

I don't think that is so. SortingAlgorithms doesn't export anything from DataStructures, so I don't really see why SortingAlgorithms would need a breaking version bump if DataStructures has a breaking version change.

Wouldn't it be way easier and cleaner to just release DataFrames 1.6.1 with the correct bounds on DataStructures, and then yank the 1.6.0 release? I can't think of any downside, it seems way less work and to me it looks like the "correct" fix for this.

I should also say that in my mind it would be perfectly OK to use these kind of runtime switches to make DataFrames work with various known DataStructures version. If DataFrames 1.6.1 has a compat for DataStructures for 0.17 and 0.18, and then uses the kind of code in this PR here, then I think that is perfectly fine.

But I really wouldn't release yet another version that looks to the package manager as if it can work with any version of DataStructures, including future ones, if we don't know that.

[bkamins]

and then yank the 1.6.0 release?

How would you technically do this? (I am OK to go the way you propose if "yanking" can be done cleanly)

[davidanthoff]

My understanding is that you just add yanked = true to the general registry for that version, like for example https://github.com/JuliaRegistries/General/blob/a911da484ee195d618613090efcff2ea8f3a01b0/E/EDF/Versions.toml#L42.

From my understanding, the package resolver will then just ignore that version going forward when it tries to resolve packages. For projects that have this version already in their manifest, it can still be instantiated. I think that is all exactly the behavior we would want here, right? It should really not be disruptive for anyone, I think.

[davidanthoff]

Only caveat is that I have never done this myself, so if you want to be 100% certain, might be worth asking in the pkg-dev channel whether this really works the way I described it above :)

[bkamins]

OK - I have updated the PR the way it was proposed and 1.6.0 version should be yanked (I hope we will learn how to do it).

I have also cleaned up Project.toml in general. I hope I done it correctly (an independent look would be welcome).

I have the following questions to make sure we made a proper Project.toml clean up while we are at it:

• @LilithHafner is SortingAlgorithms = "0.1, 0.2, 0.3, 1" compat bound OK, or maybe we should require 1 version strictly?
• @davidanthoff is IteratorInterfaceExtensions = "0.1.1, 1" and TableTraits = "0.4, 1" OK, or we should requite 1 version strictly?
• @simonster is Reexport = "0.1, 0.2, 1" OK, or we should require 1 (I think we can just require 1 as 0.1 and 0.2 were pre Julia 1.0 releases)

Thank you!

[LilithHafner]

Julia 1.6 / SortingAlgorithms 0.3.0 / DataFrames#master should be fine, so worth keeping the 0.3 compat.

SortingAlgorithms < 1.1.0 is broken on Julia >= 1.9, but the fix for that is JuliaRegistries/General#85431, so no change needed here to accommodate.

SortingAlgorithms 0.1 and 0.2 are not in the general registry, so I can't test against them using add SortingAlgorithms@0.1. I think it would make sense to drop compat with them—not worth the effort to test them against modern DataFrames imo.

[davidanthoff]

The compat entries for IteratorInterfaceExtensions and TableTraits can stay the way they are, or you could also drop the pre-1.0 versions, I'm sure it won't make any difference. But they are correct as is.

[bkamins]

I have updated the PR to require Reexport 1.x version.

[nalimilan]

Yanking 1.6.0 after tagging 1.6.1 sounds OK, but I suspect we could also simply adjust dependencies for that release in the General.jl registry. Not sure which solution is preferred, but people will probably be able to answer that if you make a PR there.

[bkamins]

Thank you!