Skip to content
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

Move towards making most public classes Final #1501

Merged
merged 6 commits into from
May 8, 2020

Conversation

njsmith
Copy link
Member

@njsmith njsmith commented May 8, 2020

This changes most of Trio's public classes so that attempts to
subclass them will produce a deprecation warning, with the idea that
once the deprecation period passes on we'll use metaclass=Final to
make subclassing an error.

See #1104 for discussion.

There are some special cases where we do allow subclassing: ABCs,
exceptions, enums, etc. I also added a test to make sure that every
public class is either Final, on its way to becoming Final, or else
falls into one of the special cases. The idea is that when we add new
classes in the future, this will force us to either mark them Final,
or else add a new special case, hopefully with a comment explaining
what makes the new class a special case.

njsmith added 4 commits May 7, 2020 14:43
These have never had a usable public constructor, so we might as well
use the annotation so in case anyone tries they'll get a nice error
message.
For when we want to switch to Final, but with a deprecation period
first.
@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #1501 into master will increase coverage by 0.00%.
The diff coverage is 98.79%.

@@           Coverage Diff           @@
##           master    #1501   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files         107      107           
  Lines       13202    13244   +42     
  Branches      995     1006   +11     
=======================================
+ Hits        13159    13201   +42     
  Misses         28       28           
  Partials       15       15           
Impacted Files Coverage Δ
trio/tests/test_exports.py 97.36% <96.66%> (+0.93%) ⬆️
trio/_core/_entry_queue.py 100.00% <100.00%> (ø)
trio/_core/_local.py 100.00% <100.00%> (ø)
trio/_core/_parking_lot.py 100.00% <100.00%> (ø)
trio/_core/_run.py 99.74% <100.00%> (ø)
trio/_core/_unbounded_queue.py 100.00% <100.00%> (ø)
trio/_highlevel_generic.py 100.00% <100.00%> (ø)
trio/_highlevel_socket.py 100.00% <100.00%> (ø)
trio/_path.py 100.00% <100.00%> (ø)
trio/_ssl.py 100.00% <100.00%> (ø)
... and 8 more

changes for fear of breaking subclasses.

There are also some classes that were explicitly designed to be
subclassed, like the ones in `trio.abc`. Subclassing these is still
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, the docs check is failing with /home/runner/work/trio/trio/docs/source/history.rst:37:py:obj reference target not found: trio.abc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, whoops, thanks for the heads up! I forgot we don't have a dedicated page for trio.abc.

@pquentin pquentin merged commit a9919d3 into python-trio:master May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants