Skip to content

Conversation

fingolfin
Copy link
Member

This is certainly a breaking change, in that it changes an explicitly documented behavior. But I suspect it won't actually break anything ; and if it does, I am glad to learn about it.

Rationale for this:

  1. I suspect it is the cause of some JET warnings
  2. returning the type Union{} for base_ring never made sense to me. Besides not fitting logically (a type is not a ring...): When would it even be used, and why?
  3. even if something relies on testing that a given ring "has no base ring" (but why???) then it makes more sense to let base_ring_type return Union{} (a method which did not exist when the "base_ring may return Union{}" convention was introduced)

Alas, I was not there when this was was designed and perhaps there are serious downsides I am not aware of -- well, as usual I am happy to learn :-).

If we merge this then of course Nemo, Hecke, ... need some changes (but I think mostly of the variety "drop base_ring method returning Union{} and adjust its base_ring_type method" )

Copy link

codecov bot commented Aug 24, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.96%. Comparing base (fea987d) to head (98a41d5).

Files with missing lines Patch % Lines
src/julia/Integer.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2144      +/-   ##
==========================================
- Coverage   87.97%   87.96%   -0.02%     
==========================================
  Files         126      126              
  Lines       31672    31669       -3     
==========================================
- Hits        27864    27858       -6     
- Misses       3808     3811       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thofma
Copy link
Member

thofma commented Aug 25, 2025

No objection from my side.

@fieker
Copy link
Contributor

fieker commented Aug 26, 2025

as long as s.o. makes sure the system continues to work... I never was a fan of Union{}, so be my guest

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

Successfully merging this pull request may close these issues.

3 participants