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

Add more convenience methods for Arb/Acb types #1703

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Conversation

fingolfin
Copy link
Member

... so that combining them with more instances of Number subtypes is accepted.

Unfortunately this is kind of an unending nightmare of things that are needlessly restricted, code duplication etc. So far I've not even touched the various acb/arb/complex/real matrix types.

Resolves #1652 (except for the discussion about normalise, for which I opened a new issue at Nemocas/AbstractAlgebra.jl#1651)

One issue with this kind of change is that it may cause method ambiguity errors. I already addressed some of those in here in a hopefully transparent manner.

Ideally this PR would add even more new test cases to cover all the new delegations, but I can't be bothered right now.

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

Attention: Patch coverage is 81.00000% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 84.74%. Comparing base (db808b2) to head (acae614).

Files Patch % Lines
src/arb/arb.jl 69.56% 7 Missing ⚠️
src/arb/Real.jl 60.00% 4 Missing ⚠️
src/arb/Complex.jl 88.88% 3 Missing ⚠️
src/arb/acb.jl 88.46% 3 Missing ⚠️
src/arb/ArbTypes.jl 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1703      +/-   ##
==========================================
+ Coverage   84.72%   84.74%   +0.01%     
==========================================
  Files          94       94              
  Lines       37469    37205     -264     
==========================================
- Hits        31746    31529     -217     
+ Misses       5723     5676      -47     

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

... so that combining them with more instances of `Number` subtypes
is accepted.
@fingolfin fingolfin marked this pull request as ready for review April 5, 2024 07:45
@fingolfin
Copy link
Member Author

To be clear: I don't expect anyone to read all of this in detail, but maybe check the gist of things... And we need more tests anyway..

Copy link
Member

@thofma thofma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works, it works

@lgoettgens lgoettgens enabled auto-merge (squash) April 5, 2024 08:07
@lgoettgens lgoettgens merged commit f033978 into master Apr 5, 2024
25 of 26 checks passed
@lgoettgens lgoettgens deleted the mh/arb-acb-compat branch April 5, 2024 09:45
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.

Method error when multiplying an AcbPolyRingElem by some small rational
3 participants