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

remove deprecated AbstractTrees calls #494

Merged
merged 1 commit into from Jun 1, 2022

Conversation

ExpandingMan
Copy link
Contributor

I've recently done a major overhaul of AbstractTrees.jl which will be part of the upcoming v0.4 release for that package. Convex.jl is unique in that it is the only registered dependency of AbstractTrees.jl which uses the soon-to-be-deprecated functions treekind and IndexedTree.

It seems that Convex.jl contains a re-implementation of print_tree because of a problem that dates back to AbstractTrees.jl v0.2. At first, I completely removed this re-implementation in favor of the current AbstractTrees implementation, but later realized that it has since diverged from AbstractTrees.print_tree even in v0.3. In particular, while maxdepth has worked fine for a while, we do not have a maxwidth. There are also some small differences in the exact printed forms.

Instead, I've removed the calls to treekind and IndexedTree. This should be fine for all versions of AbstractTree.jl because these features were very poorly documented and almost never used (which has now been explicitly verified).

@codecov
Copy link

codecov bot commented May 31, 2022

Codecov Report

Merging #494 (f45853e) into master (3d89dde) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #494   +/-   ##
=======================================
  Coverage   92.13%   92.13%           
=======================================
  Files          83       83           
  Lines        5443     5443           
=======================================
  Hits         5015     5015           
  Misses        428      428           
Impacted Files Coverage Δ
src/utilities/tree_print.jl 93.75% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d89dde...f45853e. Read the comment docs.

@odow
Copy link
Member

odow commented May 31, 2022

You happy? Minimal changes is good. At some point, Convex needs a new maintainer to go through and tidy up a lot of the package.

@ExpandingMan
Copy link
Contributor Author

Yes, should be all good.

@odow odow merged commit 35216e8 into jump-dev:master Jun 1, 2022
@baggepinnen
Copy link
Contributor

It seems Convex.jl needs a new release to include this change?
#495

@ericphanson
Copy link
Collaborator

@baggepinnen ok, registering v0.15.2

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

Successfully merging this pull request may close these issues.

None yet

4 participants