use less _element_constructor #36700
Conversation
| @@ -71,7 +71,7 @@ class IncreasingChains(RecursivelyEnumeratedSet_forest): | |||
| self._greater_than = positions | |||
| self._vertices = list(range(n)) | |||
|
|
|||
| self._element_constructor = element_constructor | |||
| self._constructor_ = element_constructor | |||
There was a problem hiding this comment.
This name _constructor_ so far does not appear anywhere in our codebase. Why is this an improvement?
There was a problem hiding this comment.
because it is now a specific name used only in this context, not related to any general mechanism
There was a problem hiding this comment.
But why use _constructor_, not _constructor?
| - ``element_constructor`` -- used to determine the type of chains, | ||
| for example ``list`` or ``tuple`` | ||
| - ``constructor`` -- used to determine the type of chains, | ||
| for example :class:`list` or :class:`tuple` |
There was a problem hiding this comment.
Can this be renamed without deprecation for the old name?
There was a problem hiding this comment.
this is not a named argument, just an argument
There was a problem hiding this comment.
All arguments are named arguments unless you use , /,. Users can do D = IncreasingChains([{0,1},{1}], element_constructor=list, exclude=[]); D
There was a problem hiding this comment.
We have rename_keyword but only for keywords, not for args. If you insist, I can make an ad-hoc warning, but I would say it's ok not to deprecate. In particular, it's not in the global namespace.
There was a problem hiding this comment.
maybe the name of the parameter can just be left as is?
fchapoton
force-pushed
the
less_elt_constructor
branch
from
2e082a9
to
3e54704
Compare
|
Documentation preview for this PR (built with commit f4a7f26; changes) is ready! 🎉 |
trying to avoid ` _element_constructor` with no final underscore
the only serious change is the one in finite fields morphisms.
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
URL: sagemath#36700
Reported by: Frédéric Chapoton
Reviewer(s): Frédéric Chapoton, Matthias Köppe
trying to avoid
_element_constructorwith no final underscorethe only serious change is the one in finite fields morphisms.
📝 Checklist