-
-
Notifications
You must be signed in to change notification settings - Fork 672
convert some methods in designs and graphs to n_* #40887
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
base: develop
Are you sure you want to change the base?
Conversation
Documentation preview for this PR (built with commit 5ae07a9; changes) is ready! 🎉 |
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.
Thanks for the contribution!
If you want the aliases to be deprecated eventually you can use this: https://doc.sagemath.org/html/en/reference/misc/sage/misc/superseded.html#sage.misc.superseded.deprecated_function_alias
If you don't want the aliases to get deprecated then this is fine. My suggestion to change the functions that return int
to Integer
is an "if you want to" suggestion, this PR is already an improvement and I won't insist on expanding the scope.
I'm happy with this as-is, if you are too feel free to set to positive review. If you want to make the additional changes I suggested (marking the functions as deprecated aliases or changing int
to Integer
) then tag me when it's ready for review again.
sage: designs.DesarguesianProjectivePlaneDesign(2).num_points() | ||
7 | ||
""" | ||
return len(self._points) |
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.
Is there a reason this returns int
instead of Integer
? If not, you could change this to return an Integer
.
I just saw the discussion in #40875 about deprecations, I take back my suggestion to deprecate the aliases since there are some good reasons to not deprecate these names. If you want to make the |
thanks for the review. I choose to set to positive. |
@fchapoton, will you also do the remaining
|
This replaces methods
num_points
andnum_blocks
byn_points
andn_blocks
in designs.Old names are kept as aliases.
cf also #40875
This is just a small reduction of the number of method with names
num_*
📝 Checklist