typing annotation for gens method in rings and groups (pyx files) #39476
Conversation
|
Documentation preview for this PR (built with commit 450d6f6; changes) is ready! 🎉 |
| @@ -362,7 +362,7 @@ class ParentLibGAP(SageObject): | |||
| for gap_subgroup in self._libgap.MaximalNormalSubgroups()] | |||
|
|
|||
| @cached_method | |||
| def gens(self): | |||
| def gens(self) -> tuple: | |||
There was a problem hiding this comment.
This comment applies to all your uses of the tuple/list annotation in this PR: for any of these, are you able to specify what type the tuple/list is composed of? Even if you can't get more precise than a fairly general class like Element, it would still be better than just a generic tuple. You can specify a tuple of for example three Elements by tuple[Element, Element, Element] or an unknown number of Elements by tuple[Element, ...]. Of course the most precise object you can use would be ideal. If there are really absolutely no guarantees on the type then I guess that's fine (but surely at least SageObject would work?).
| """ | ||
| return [self.gen()] | ||
| return (self.gen(),) |
There was a problem hiding this comment.
While a tuple is probably a better choice here, is this not technically a breaking change? Since tuples have less capabilities than a list (since they are immutable)? Not sure what the deprecation rules would be here, or if they even apply.
There was a problem hiding this comment.
Thanks for the comments.
The aim is to make sure that all .gens method everywhere in sage all return either tuple or Family.
This may not be possible, but at least it is worth trying.
The annotation here is mostly to remember easily that this property has been checked. Refining this annotation would be better done in another occasion.
There was a problem hiding this comment.
I think what you wrote here addresses the other comment I made. I don't think it addresses my question of whether this is a breaking change.
just adding annotation to remember what has been checked
📝 Checklist