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

Clean up HeckeMiscInteger.jl #1848

Merged
merged 12 commits into from
Sep 20, 2024
Merged

Clean up HeckeMiscInteger.jl #1848

merged 12 commits into from
Sep 20, 2024

Conversation

joschmitt
Copy link
Collaborator

The behaviour of mod_sym is still open for debate.

Copy link

codecov bot commented Sep 17, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 23 lines in your changes missing coverage. Please review.

Project coverage is 87.06%. Comparing base (3e66d55) to head (e0b7461).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
src/flint/fmpz.jl 83.21% 23 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1848      +/-   ##
==========================================
+ Coverage   86.19%   87.06%   +0.86%     
==========================================
  Files          98       98              
  Lines       36210    40661    +4451     
==========================================
+ Hits        31212    35400    +4188     
- Misses       4998     5261     +263     

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

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Thanks


^(a::T, n::IntegerUnion) where {T<:RingElem} = _generic_power(a, n)

function _generic_power(a, n::IntegerUnion)
Copy link
Member

Choose a reason for hiding this comment

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

This is not really specific to ZZRingElem. I wonder if we should have a src/generic.jl file or perhaps src/generic dir in which we move such "generic" code (I think we have more examples in this vein).

Anyway, also OK by me to move it here for now. Just food for thought (since you've been touching many of these files, you probably have a better feeling than me whether this idea has merit or not)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are some functions where it is not really clear, where they should go. In the end, I wouldn't know where I would search for a generic powering function, so I would need git grep anyway...

@lgoettgens lgoettgens closed this Sep 20, 2024
@lgoettgens lgoettgens reopened this Sep 20, 2024
@fingolfin fingolfin merged commit 2ead0d2 into Nemocas:master Sep 20, 2024
47 of 48 checks passed
@joschmitt joschmitt deleted the js/miscint branch September 22, 2024 13:50
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.

4 participants