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

Mark abstract base classes and methods #4503

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4503.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Mark abstract base classes and methods with `abc.ABC` and `abc.abstractmethod` -- by :user:`Avasam`
3 changes: 2 additions & 1 deletion pkg_resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from __future__ import annotations

from abc import ABC
import sys

if sys.version_info < (3, 8): # noqa: UP036 # Check for unsupported versions
Expand Down Expand Up @@ -311,7 +312,7 @@ def get_supported_platform():
]


class ResolutionError(Exception):
class ResolutionError(Exception, ABC):
"""Abstract base for dependency resolution errors"""

def __repr__(self):
Expand Down
65 changes: 38 additions & 27 deletions setuptools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

from abc import ABC, abstractmethod
import functools
import os
import re
Expand Down Expand Up @@ -119,7 +120,7 @@ def setup(**attrs):
_Command = monkey.get_unpatched(distutils.core.Command)


class Command(_Command):
class Command(_Command, ABC):
"""
Setuptools internal actions are organized using a *command design pattern*.
This means that each action (or group of closely related actions) executed during
Expand All @@ -132,42 +133,25 @@ class Command(_Command):
When creating a new command from scratch, custom defined classes **SHOULD** inherit
from ``setuptools.Command`` and implement a few mandatory methods.
Between these mandatory methods, are listed:

.. method:: initialize_options(self)

Set or (reset) all options/attributes/caches used by the command
to their default values. Note that these values may be overwritten during
the build.

.. method:: finalize_options(self)

Set final values for all options/attributes used by the command.
Most of the time, each option/attribute/cache should only be set if it does not
have any value yet (e.g. ``if self.attr is None: self.attr = val``).

.. method:: run(self)

Execute the actions intended by the command.
(Side effects **SHOULD** only take place when ``run`` is executed,
for example, creating new files or writing to the terminal output).
:meth:`initialize_options`, :meth:`finalize_options` and :meth:`run`.

A useful analogy for command classes is to think of them as subroutines with local
variables called "options". The options are "declared" in ``initialize_options()``
and "defined" (given their final values, aka "finalized") in ``finalize_options()``,
variables called "options". The options are "declared" in :meth:`initialize_options`
and "defined" (given their final values, aka "finalized") in :meth:`finalize_options`,
both of which must be defined by every command class. The "body" of the subroutine,
(where it does all the work) is the ``run()`` method.
Between ``initialize_options()`` and ``finalize_options()``, ``setuptools`` may set
(where it does all the work) is the :meth:`run` method.
Between :meth:`initialize_options` and :meth:`finalize_options`, ``setuptools`` may set
the values for options/attributes based on user's input (or circumstance),
which means that the implementation should be careful to not overwrite values in
``finalize_options`` unless necessary.
:meth:`finalize_options` unless necessary.

Please note that other commands (or other parts of setuptools) may also overwrite
the values of the command's options/attributes multiple times during the build
process.
Therefore it is important to consistently implement ``initialize_options()`` and
``finalize_options()``. For example, all derived attributes (or attributes that
Therefore it is important to consistently implement :meth:`initialize_options` and
:meth:`finalize_options`. For example, all derived attributes (or attributes that
depend on the value of other attributes) **SHOULD** be recomputed in
``finalize_options``.
:meth:`finalize_options`.

When overwriting existing commands, custom defined classes **MUST** abide by the
same APIs implemented by the original class. They also **SHOULD** inherit from the
Expand Down Expand Up @@ -238,6 +222,33 @@ def reinitialize_command(
vars(cmd).update(kw)
return cmd

@abstractmethod
def initialize_options(self) -> None:
"""
Set or (reset) all options/attributes/caches used by the command
to their default values. Note that these values may be overwritten during
the build.
"""
raise NotImplementedError

@abstractmethod
def finalize_options(self) -> None:
"""
Set final values for all options/attributes used by the command.
Most of the time, each option/attribute/cache should only be set if it does not
have any value yet (e.g. ``if self.attr is None: self.attr = val``).
"""
raise NotImplementedError

@abstractmethod
def run(self) -> None:
"""
Execute the actions intended by the command.
(Side effects **SHOULD** only take place when :meth:`run` is executed,
for example, creating new files or writing to the terminal output).
"""
raise NotImplementedError

Copy link
Contributor Author

@Avasam Avasam Jul 21, 2024

Choose a reason for hiding this comment

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

Is it possible to for the class' doc to reference its own methods in rst ? As to not duplicate the descriptions ?

    .. method:: initialize_options(self)
        Set or (reset) all options/attributes/caches used by the command
        to their default values. Note that these values may be overwritten during
        the build.
    .. method:: finalize_options(self)
        Set final values for all options/attributes used by the command.
        Most of the time, each option/attribute/cache should only be set if it does not
        have any value yet (e.g. ``if self.attr is None: self.attr = val``).
    .. method:: run(self)
        Execute the actions intended by the command.
        (Side effects **SHOULD** only take place when ``run`` is executed,
        for example, creating new files or writing to the terminal output).

Edit: The docs do show a warning about this: https://github.com/pypa/setuptools/actions/runs/10031767614/job/27722602923?pr=4503#step:5:93

Copy link
Contributor

@abravalheri abravalheri Aug 8, 2024

Choose a reason for hiding this comment

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

Probably yes. We are using autocommand magic in: https://github.com/pypa/setuptools/blob/main/docs/userguide/extension.rst?plain=1#L78-L89. So I guess we have to remove the .. method:: definition and instead just mention the names of the methods in the text. I think autocommand will already include the docstring (due to the :members: option).

Might require some text adjustments, but even if we have to mention them without a link just with double backticks, it should be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this part of the docstring and replace with a simple mention in this PR?

    .. method:: initialize_options(self)
        Set or (reset) all options/attributes/caches used by the command
        to their default values. Note that these values may be overwritten during
        the build.
    .. method:: finalize_options(self)
        Set final values for all options/attributes used by the command.
        Most of the time, each option/attribute/cache should only be set if it does not
        have any value yet (e.g. ``if self.attr is None: self.attr = val``).
    .. method:: run(self)
        Execute the actions intended by the command.
        (Side effects **SHOULD** only take place when ``run`` is executed,
        for example, creating new files or writing to the terminal output).

(Just to avoid the duplication in the docs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice, I can easily self-reference without having to include module and class names.


def _find_all_simple(path):
"""
Expand Down
7 changes: 6 additions & 1 deletion setuptools/command/setopt.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from abc import ABC, abstractmethod
from distutils.util import convert_path
from distutils import log
from distutils.errors import DistutilsOptionError
Expand Down Expand Up @@ -68,7 +69,7 @@ def edit_config(filename, settings, dry_run=False):
opts.write(f)


class option_base(Command):
class option_base(Command, ABC):
"""Abstract base class for commands that mess with config files"""

user_options = [
Expand Down Expand Up @@ -103,6 +104,10 @@ def finalize_options(self):
)
(self.filename,) = filenames

@abstractmethod
def run(self) -> None:
raise NotImplementedError


class setopt(option_base):
"""Save command-line options to a file"""
Expand Down
3 changes: 2 additions & 1 deletion setuptools/sandbox.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from abc import ABC
import os
import sys
import tempfile
Expand Down Expand Up @@ -265,7 +266,7 @@ def run_setup(setup_script, args):
# Normal exit, just return


class AbstractSandbox:
class AbstractSandbox(ABC):
"""Wrap 'os' module and 'open()' builtin for virtualizing setup scripts"""

_active = False
Expand Down
Loading