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

Sort out deprecations and aliases #1458

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

lgoettgens
Copy link
Contributor

This needs Nemocas/Nemo.jl#1704 to be available, and the Nemo compat to be adapted accordingly.

This is an approach to sort deprecations and aliases as discussed with @fingolfin and already started in Nemocas/AbstractAlgebra.jl#1537 and Nemocas/Nemo.jl#1704.
There are some workarounds added to make this non-breaking for Oscar 1.0, that should get removed in the next minor release.

Things that have been changed:

  • put all permanent aliases into aliases.jl, remove duplicates with AA (ishermitian)
  • collect all deprecations in three lists in deprecations.jl (temp aliases, deprecate_bindings, and deprecations)
  • no longer use path to import stuff from Nemo, instead call @include_deprecated_bindings() from both AA and Nemo. The initial problem both of these approaches fix is that types that are deprecated using @deprecate_binding are not re-exported (but should). @fingolfin and I found a macro the easiest way to execute the deprecations again in the Hecke scope (and once everything is updated in the Oscar scope as well).

@lgoettgens lgoettgens force-pushed the lg/deprecations-aliases branch from ff2bc5e to 6af8c52 Compare April 24, 2024 14:36
@lgoettgens lgoettgens closed this Apr 24, 2024
@lgoettgens lgoettgens reopened this Apr 24, 2024
@lgoettgens lgoettgens marked this pull request as ready for review April 24, 2024 14:54
@lgoettgens lgoettgens closed this Apr 25, 2024
@lgoettgens lgoettgens reopened this Apr 25, 2024
Copy link

codecov bot commented Apr 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 6.59%. Comparing base (8048731) to head (6af8c52).

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1458       +/-   ##
===========================================
- Coverage   75.20%    6.59%   -68.62%     
===========================================
  Files         354      354               
  Lines      112687   112530      -157     
===========================================
- Hits        84746     7416    -77330     
- Misses      27941   105114    +77173     
Files Coverage Δ
src/Deprecations.jl 33.33% <100.00%> (+33.33%) ⬆️
src/Hecke.jl 32.48% <ø> (-11.14%) ⬇️

... and 313 files with indirect coverage changes

@lgoettgens
Copy link
Contributor Author

Bump @thofma.
If there are any concerns with this please let me know. It would be great to get this out in a non-breaking release before the large chain of breaking releases reaches Hecke, to make the handling downstream a bit easier, and to be able to already prepare for the following breaking changes.

@thofma thofma enabled auto-merge (squash) April 29, 2024 09:16
@thofma thofma disabled auto-merge April 29, 2024 12:13
@thofma thofma merged commit 8ddf481 into thofma:master Apr 29, 2024
36 of 50 checks passed
@lgoettgens lgoettgens deleted the lg/deprecations-aliases branch April 29, 2024 12:48
@lgoettgens
Copy link
Contributor Author

Thanks for merging. Could you release this as a patch release so I can already start adapting Oscar?

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.

2 participants