-
Notifications
You must be signed in to change notification settings - Fork 63
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
is_finiteorder
-> is_finite_order
#1639
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1639 +/- ##
=======================================
Coverage 86.76% 86.76%
=======================================
Files 114 114
Lines 29693 29693
=======================================
Hits 25763 25763
Misses 3930 3930 ☔ View full report in Codecov by Sentry. |
I think it would be better for Oscar to make an alias for now and do the deprecation later? |
Yes, I wasn't sure whether this would break OSCAR but unfortunately it does. |
55fcbc8
to
44958b6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am for this change, and in fact we discussed it when breaking away for GroupsCore but for various reasons decided to do it later (and then forgot, I guess).
Of course it should be done in a way that maximizes compatibility / minimizes (or better: avoids) breakage.
However, I thought @alias
would do that, and in fact OscarCI tests pass, so what is the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem anymore, when I made my comment it was realized as a deprecation (https://github.com/Nemocas/AbstractAlgebra.jl/compare/55fcbc8a86e68ab49406fc7cc5b2a1d505ea55a0..44958b69cdf32ece70510fd5edac2ed91c897e97) but that is changed now.
Too late, but what about the |
I am more in favor of is_finite_order, since for the GAP functions in Oscar (of which there are a lot of around groups), there always is an additional has_ and set_. I think this could lead to too much confusion if now has_finite_order does not check if finite_order is set (as the GAP functions would do).
…On March 22, 2024 9:49:50 AM GMT+01:00, Johannes Schmitt ***@***.***> wrote:
Too late, but what about the `is_finite_order` vs `has_finite_order`?
--
Reply to this email directly or view it on GitHub:
#1639 (comment)
You are receiving this because you modified the open/close state.
Message ID: ***@***.***>
|
I think
is_finiteorder
is against the naming conventions? While I'm at it: shouldn't it behas_finite_order
?