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.

Sign up for GitHub

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

Jump to bottom

Rename number_of_* functions consistently #1553

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Rename number_of_* functions consistently #1553

merged 7 commits into from
Jan 23, 2024

Conversation

lgoettgens
Copy link
Collaborator

See https://hackmd.io/nRnyfrSxTVe5CzXidB1cBQ?both#number-of-%E2%80%A6

Strategy while renaming:

  • Changed: doc pages, docstrings, function definitions
  • kept as is: most uses (modulo some few misclicks while search-and-replacing)

I couldn't put the Aliases into the Aliases.jl file, as that gets loaded to late to use the shorthands in AA. Thus, I instead created function stubs with an alias in AbstractAlgebra.jl in the beginning.

All changes here should be completely non-breaking, as all old names are still there as an alias.

@codecov Codecov
Copy link

codecov bot commented Jan 18, 2024 •
edited
Loading

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (5a441df) 87.11% compared to head (bff0247) 87.11%.
Report is 1 commits behind head on master.

❗ Current head bff0247 differs from pull request most recent head e5c56b5. Consider uploading reports for the commit e5c56b5 to get more accurate results

Files Patch % Lines
src/Matrix.jl 0.00% 2 Missing ⚠️
src/julia/Matrix.jl 0.00% 2 Missing ⚠️
src/Groups.jl 0.00% 1 Missing ⚠️
src/MPoly.jl 0.00% 1 Missing ⚠️
src/NemoStuff.jl 0.00% 1 Missing ⚠️
src/generic/AbsMSeries.jl 50.00% 1 Missing ⚠️
src/generic/Fraction.jl 0.00% 1 Missing ⚠️
src/generic/LaurentMPoly.jl 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1553   +/-   ##
=======================================
  Coverage   87.11%   87.11%           
=======================================
  Files         114      114           
  Lines       29483    29483           
=======================================
  Hits        25685    25685           
  Misses       3798     3798

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

@lgoettgens lgoettgens mentioned this pull request Jan 18, 2024
Merged
@lgoettgens lgoettgens closed this Jan 18, 2024
@lgoettgens lgoettgens reopened this Jan 18, 2024
fingolfin
fingolfin reviewed Jan 22, 2024
View reviewed changes
src/AbstractAlgebra.jl Outdated
@@ -15,7 +15,7 @@ using Preferences
using Test # for "interface-conformance" functions

import GroupsCore
import GroupsCore: gens, ngens, order, mul!, istrivial
import GroupsCore: gens, order, mul!, istrivial
Copy link
Member

Choose a reason for hiding this comment

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

I guess we need to decide how to order this WRT to the GroupsCore removal PR...

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 already is a comment somewhere below on how to chamge this PR after the group score one has been merged. So this will get a small update after the merge of #1528

fingolfin
fingolfin approved these changes Jan 22, 2024
View reviewed changes
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.

Looks good to me (but depending on whether this or the GroupsCore PR is merged first, we need to adjust one or the other)

@lgoettgens
Copy link
Collaborator Author

this is now ready to get merged @thofma @fingolfin

@thofma
Copy link
Member

thofma commented Jan 23, 2024

Is nrows and ncols never mentioned anywhere in the docs?

@lgoettgens
Copy link
Collaborator Author

Is nrows and ncols never mentioned anywhere in the docs?

only in docs/src/matrix.md and in some few further docstrings (I looked trough all matches in vscode and changed all docstrings)

thofma
thofma reviewed Jan 23, 2024
View reviewed changes
src/Matrix.jl Outdated Show resolved Hide resolved
@lgoettgens @thofma
Update src/Matrix.jl [skip ci]
e5c56b5 
Co-authored-by: Tommy Hofmann <[email protected]>
@thofma thofma merged commit 1a79eb7 into Nemocas:master Jan 23, 2024
@lgoettgens lgoettgens deleted the lg/rename-number-of branch January 23, 2024 11:26
This was referenced Jan 24, 2024
Merged
Merged
@lgoettgens lgoettgens mentioned this pull request Feb 2, 2024
Merged
ooinaruhugh pushed a commit to ooinaruhugh/AbstractAlgebra.jl that referenced this pull request Feb 15, 2024
@lgoettgens @thofma @ooinaruhugh
Rename number_of_* functions consistently (Nemocas#1553)
4e07e61 
* ncols

* ndigits

* ngens

* nrows

* nvars

* Update src/Matrix.jl [skip ci]

Co-authored-by: Tommy Hofmann <[email protected]>

---------

Co-authored-by: Tommy Hofmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thofma thofma thofma left review comments

@fingolfin fingolfin fingolfin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lgoettgens @thofma @fingolfin