-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Draft documentation for final attributes #5754
Conversation
Do you want this cherry-picked in the 0.640 release branch? |
I think that we can cherry-pick this if we have a non-draft version before the release. I'll finish this up tomorrow. |
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 have a few mutterings but apart from the typo(s) this could also go in as-is. I'll want to quote from and link to this in the release blog post.
def __init__(self) -> None: | ||
self.id: Final = uuid.uuid4() | ||
|
||
# 1000 lines later |
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.
This comment feels irrelevant.
.. note:: | ||
|
||
This is an experimental feature. Some details might change in later | ||
versions of mypy. The final qualifiers are available in the |
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.
Maybe instead of "the final qualifiers" write "the final
decorator and Final
pseudo-type"?
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.
We discussed this with Jukka, and IIRC he suggested just "qualifiers".
Syntax variants | ||
*************** | ||
|
||
The ``typing_extensions.Final`` qualifier indicates that a given name or |
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.
So I'm not sure that "qualifier" is the best term here, though I'm not sure that "pseudo-type" is any better. We probably should use the same word to describe "ClassVar" (see #5733).
attribute should never be re-assigned, re-defined, nor overridden. It can be | ||
used in one of these forms: | ||
|
||
|
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.
Only one blank line.
``ID: Final[float]``. | ||
|
||
* Finally, you can define ``self.id: Final = 1`` (also with a type argument), | ||
but this is allowed *only* in ``__init__`` methods. |
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.
Why the limitation to __init__
methods? In general we allow inferring or declaring the type of instance variables in any method (though IIRC a declaration must be on the first use). Is this because the implementation cannot ensure that there aren't any assignments in other places if Final
isn't set in __init__
?
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.
Because the instance attribute should be set only once, when the instance is created. I think we should also allow this in __new__
, but this is blocked by #1021. I will try to reword here to make this clear and add a follow-up issue for __new__
.
|
||
.. code-block:: python | ||
|
||
from typing_extensions import final |
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.
Add a comment emphasizing this uses a lower-case 'f'.
def common_name(self) -> None: | ||
... | ||
|
||
# 1000 lines later |
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.
Drop.
y.append('x') # Error: Sequance is immutable | ||
z: Final = ('a', 'b') # Also an option | ||
|
||
Final methods |
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.
Ironic that here "Final" is capitalized even though in the code it uses a lower-case 'f', whereas in previous section headings "final" was not capitalized but the code used Final
. :-) Maybe add "(the @final
decorator)" to the title here?
def meth(self, x=None): | ||
... | ||
|
||
Final classes |
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.
Add "(the @final
class decorator)"?
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 not sure this is needed, we can add this later.
in the future, and these changes might break subclasses. | ||
* You believe that subclassing would make code harder to understand or maintain. | ||
For example, you may want to prevent unnecessarily tight coupling between | ||
base classes and subclasses. |
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.
These bullets all make almost the same point. Maybe we don't need to editorialize and users can figure out for themselves why final classes might be useful (they might even know the concept from another language).
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.
IIRC, Jukka proposed to add some examples, and I think it probably makes sense to keep at least some of them.
Draft work towards #5598. I am merging this now so I'll have something to link to from the blog post. We can polish it later.
Draft work towards #5598
@JukkaL you can either push directly to this branch or merge this and polish it afterwards.