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

v0.4 dependents compatiblity #103

Open
ExpandingMan opened this issue May 29, 2022 · 5 comments
Open

v0.4 dependents compatiblity #103

ExpandingMan opened this issue May 29, 2022 · 5 comments

Comments

@ExpandingMan
Copy link
Contributor

ExpandingMan commented May 29, 2022

v0.4 Eco Checks

I have run unit tests for all direct dependents against v0.4 as of the latest commit on my latest PR.

As you can see, most packages nearly require a compat bound bump. A few are using the deprecated
function parentlinks and likely require only a simple change.

At the moment it seems there are only two non-trivial cases:

  • BehaviorTree: this uses ShadowTree, a data structure which I did not understand the reason for
    in the first place. I expect that whatever it was doing can be easily replaced, but I'll have
    to take a look at this package to see what the v0.4 equivalent will be.
  • Convex: this uses treekind which suggests that it is one of the only packages using indexed
    trees, and therefore is likely to require a non-trivial update.

AutocorrelationShell

BasisFunctions

factorial(::Float64) method error that happens even for v0.3

BehaviorTree

uses ShadowTree

Casciadia

CombinedParsers

ConstituencyTrees

ConstructiveGeometry

Unrelated error, same thing happens on v0.3

Convex

treekind not defined

D3Trees

DataDepsGenerators

unrelated error, same thing on v0.3

DataSets

DecisionTree

DocumentationGenerator

unrelated error, same thing on v0.3

DocumenterTools

unrelated error, same thing on v0.3

ExprOptimization

ExprRules

ExtendableGrids

unrelated error, same thing on v0.3

Eyeball

FeynmanDiagram

I'm getting a parquet-related test error, but exactly same error on v0.3.

FileTreees

FlameGraphs

parentlinks not defined

FoldingTrees

Flux

GenericTensorNetworks

unrelated error, same thing on v0.3, lots of passing tests

GraphRecipes

Gridap

GridapEmbedded

Gumbo

HMatrices

HalfEdges

unrelated error, same thing on v0.3

HssMatrices

ITensors

ImageComponentAnalysis

expects parentlinks

InfiniteOpt

expects parentlinks

InteractiveErrors

JSServe

Seems to have unrelated errors, but testing is a dependency mess, not completely sure.

JunctionTrees

KeywordSearch

LeftChildRightSiblingTrees

expects parentlinks

MathML

MathTexEngine

MetaUtils

MethodAnalysis

ModelingToolkit

depends on LeftChildRightSiblingTrees

MultiScaleTreeGraph

expects parentlinks

NewickTree

expects Tree

OMEinsum

OnlineStats

OnlineStatsBase

PDFIO

PProf

depends on LeftChildRightSiblingTrees

PSID

unrelated error, trying to fetch remote stuff

Parquet2

PatchMixtureKriging

PhysOcean

PlutoProfile

depends on LeftChildRightSiblingTrees

ProjectEulerUtil

QXContexts

seems to have packaging issues

QXTools

ReinforcementLearningBase

ReinforcementLearningCore

ReinforcementLearningExperiments

ReinforcementLearningZoo

Remarkable

requires 3rd party remote service to test

RewriteTools

SIIPExamples

depends on LeftChildRightSiblingTrees

Scruff

Skans

SymbolicUtils

depends on LeftChildRightSiblingTrees but still passes

TSML

Taxonomy

TimeDag

unrelated error, same thing on v0.3, passes lots of tests

Transformers

TreesHeaps

expects parentlinks

Tries

UnROOT

WRDSMerger

expcts parentlinks

WavePropBase

Wordlegames

XML

@ExpandingMan
Copy link
Contributor Author

I have looked into the first of the non-trivial breaks, and made this PR to BehaviorTree.jl.

It appears the authors were not really using ShadowTree, which support the decision to deprecate it. I suspect authors had in mind some use for it only to discover it was not very useful after all.

As far as I can tell, this "solves" ShadowTree. No other packages use it, and we haven't found any uses of it which would not be better served another way.

@ExpandingMan
Copy link
Contributor Author

I have now investigated the other unique case, that of Convex.jl which was using treekind and IndexedTree. I have fixed their usage to be compatible with both v0.3 and the upcoming v0.4, the result of which can be seen in this PR.

Convex.jl is in a rather strange situation in that it is maintaining a completely separate but similar re-implementation of print_tree which apparently dates back to bugs in AbstractTrees.jl that were solved long ago. I was about to remove their implementation entirely but it seems to have diverged since. It would be better for both packages if the implementations were unified, but frankly I don't personally care that much about printing so I didn't feel like taking it on.

@ExpandingMan
Copy link
Contributor Author

See also this PR which updates LeftChildRightSibling trees.

@oscardssmith
Copy link
Member

are we ready to bump the version and start the (painful) rollout?

@ExpandingMan
Copy link
Contributor Author

I'd say might as well just go ahead and do it. The only thing I can think of that we are definitely lacking is that we clearly need better unit test coverage. I think the most commonly used cases will be fine, but we only have around 2/3 test coverage. Might not be a good reason to hod back, however, since it will take a little while for packages to update their bounds. It would take a major catastrophe for increasing test coverage to result in a breaking version.

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

No branches or pull requests

2 participants