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

Consistently use ZZ/QQ instead of mix of ZZ/QQ and FlintZZ/FlintQQ #1718

Merged
merged 5 commits into from
Apr 22, 2024

Conversation

lgoettgens
Copy link
Collaborator

@lgoettgens lgoettgens commented Apr 17, 2024

To make the code more consistent and readable. The objects FlintZZ and FlintQQ stay for compatibility reasons.

@lgoettgens lgoettgens force-pushed the lg/FlintQQZZ branch 2 times, most recently from 03d42cb to ba5f1d5 Compare April 17, 2024 14:21
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

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

Project coverage is 84.90%. Comparing base (023d385) to head (ae30d4e).

Files Patch % Lines
src/HeckeMoreStuff.jl 11.11% 8 Missing ⚠️
src/HeckeMiscMatrix.jl 25.00% 6 Missing ⚠️
src/HeckeMiscPoly.jl 0.00% 2 Missing ⚠️
src/flint/fmpq_mpoly.jl 33.33% 2 Missing ⚠️
src/flint/fmpz.jl 75.00% 2 Missing ⚠️
src/flint/fmpz_mpoly.jl 33.33% 2 Missing ⚠️
src/flint/padic.jl 75.00% 1 Missing ⚠️
src/flint/qadic.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1718      +/-   ##
==========================================
- Coverage   84.92%   84.90%   -0.02%     
==========================================
  Files          95       95              
  Lines       37235    37235              
==========================================
- Hits        31620    31614       -6     
- Misses       5615     5621       +6     

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

@lgoettgens lgoettgens closed this Apr 18, 2024
@lgoettgens lgoettgens reopened this Apr 18, 2024
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.

Presumably this was done to 100% exclude any chance of confusion as to which QQ and ZZ is used, to catch any mistakes where perhaps the alternative definitions in AA get imported by accident somewhere.

But it seems to me we have established quite clearly this is not the case, so I am OK with making this change...

@fingolfin fingolfin merged commit 22e3908 into Nemocas:master Apr 22, 2024
23 of 26 checks passed
@lgoettgens lgoettgens deleted the lg/FlintQQZZ branch April 22, 2024 13:51
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.

None yet

2 participants