-
-
Notifications
You must be signed in to change notification settings - Fork 710
Improve documentation of various gap-related methods #39905
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
Conversation
f32e829 to
fbc3629
Compare
fbc3629 to
f29bd6d
Compare
|
Documentation preview for this PR (built with commit 4b238ad; changes) is ready! 🎉 |
| Return a Gap object. | ||
| Unlike :meth:`_libgap_`, this method returns an instance of | ||
| :class:`sage.interfaces.gap.GapElement`, which wraps an object |
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'm not quite sure if this is the intention, or maybe this can return an instance of sage.libs.gap.libgap.GapElement if the argument G is libgap — with careful implementation, the method can be made to work in both cases.
Not sure what's the advantage of doing that however. If I recalled correctly some other parts of the code also did that.
|
looks good to me. I will wait for the CI |
fchapoton
left a comment
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.
ok, let's move on
As in the title.
There will be no visible change to the user because these methods all start with
_, only to Sage developers.The reason is… there's a lot of confusion in downstream implementations, so it looks like whoever initially wrote that code wasn't aware of the convention. See #39908 .
📝 Checklist
⌛ Dependencies