Skip to content

Revise docstrings, comments, and a few messages #1850

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

Merged
merged 61 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
9b5531b
Fix typos and further clarify Git.refresh docstring
EliahKagan Feb 24, 2024
c0cd8a8
Clarify comment on shell case in _safer_popen_windows
EliahKagan Feb 24, 2024
afd943a
Tweak message about GIT_PYTHON_REFRESH for 80-column terminals
EliahKagan Feb 24, 2024
e08066c
Revise docstrings in git.__init__ and git.cmd
EliahKagan Feb 24, 2024
8bb882e
Fix concurrency note for stream_object_data
EliahKagan Feb 24, 2024
ba878ef
Reword partial_to_complete_sha_hex note
EliahKagan Feb 24, 2024
3958747
Update CommandError._msg documentation
EliahKagan Feb 24, 2024
f56e1ac
Tweak code formatting in Remote._set_cache_
EliahKagan Feb 24, 2024
fa471fe
Fix up Remote.push docstring
EliahKagan Feb 24, 2024
1cd73ba
Revise docstrings in second-level modules
EliahKagan Feb 25, 2024
29c63ac
Format first Git.execute overload stub like the others
EliahKagan Feb 25, 2024
cd8a312
Show full-path refresh() in failure message differently
EliahKagan Feb 25, 2024
8ec7e32
Revise docstrings within git.index
EliahKagan Feb 26, 2024
ca2ab61
Rewrite post_clear_cache note
EliahKagan Feb 26, 2024
37421e1
Further revise post_clear_cache docstring
EliahKagan Feb 26, 2024
ca32c22
Condense output_stream description in Git.execute docstring
EliahKagan Feb 26, 2024
3813bfb
Clarify Submodule.branch_path documentation
EliahKagan Feb 26, 2024
63c62ed
Revise docstrings within git.objects.submodule
EliahKagan Feb 27, 2024
115451d
Change _write to write in SubmoduleConfigParser docstring
EliahKagan Feb 27, 2024
5219489
Fix IndexObject.abspath docstring formatting
EliahKagan Feb 27, 2024
c06dfd9
Fix parameter names in TagObject.__init__
EliahKagan Feb 27, 2024
ae37a4a
Revise docstrings within git.objects
EliahKagan Feb 27, 2024
37011bf
Fix backslash formatting in git.util docstrings
EliahKagan Feb 27, 2024
d9fb2f4
Further git.util docstring revisions
EliahKagan Feb 27, 2024
d1d18c2
Revise docstrings within git.refs
EliahKagan Feb 27, 2024
4f67369
Fix backslashes in Repo.__init__ docstring
EliahKagan Feb 27, 2024
0c8ca1a
Fix Repo.iter_commits docstring about return type
EliahKagan Feb 27, 2024
b2b6f7c
Revise docstrings within git.repo
EliahKagan Feb 28, 2024
c67b2e2
Adjust spacing in colon seach mode NotImplementedError
EliahKagan Feb 28, 2024
5ee8744
Update git source link in Repo.merge_base comment
EliahKagan Feb 28, 2024
c8b6cf0
Update comment about improving expand_path overloads
EliahKagan Feb 28, 2024
bcc0c27
Fix recent inconsistency, using :raise:, not :raises:
EliahKagan Feb 28, 2024
0231b74
Further revise docstrings in git.objects.submodule.base
EliahKagan Feb 28, 2024
8344f44
Revise Repo.archive docstring
EliahKagan Feb 28, 2024
432ec72
Fix another :raises: to :raise:
EliahKagan Feb 28, 2024
5ca5844
Fully qualify non-builtin exceptions in :raise:
EliahKagan Feb 28, 2024
e6768ec
Improve Git.execute docstring formatting re: max_chunk_size
EliahKagan Feb 28, 2024
cd61eb4
Further revise docstrings in second-level modules
EliahKagan Feb 28, 2024
fc1762b
Undo a couple minor black-incompatible changes
EliahKagan Feb 28, 2024
1b25a13
Further revise docstrings within git.index
EliahKagan Feb 28, 2024
08a80aa
Further revise docstrings within git.objects.submodule
EliahKagan Feb 28, 2024
bc48d26
Further revise other docstrings within git.objects
EliahKagan Feb 28, 2024
30f7da5
Fix erroneous reference to DateTime "class"
EliahKagan Feb 28, 2024
6126997
Improve docstrings about tags
EliahKagan Feb 29, 2024
110706e
Fix param name in TagRefernece docstring and add info
EliahKagan Feb 29, 2024
b0e5bff
Undo some expansion of "reference" parameter
EliahKagan Feb 29, 2024
a5a1b2c
Add a bit of missing docstring formatting
EliahKagan Feb 29, 2024
018ebaf
Further revise docstrings within git.repo
EliahKagan Feb 29, 2024
608147e
Better explain conditional cleanup in test_base_object
EliahKagan Feb 29, 2024
5cf5b60
Revise test suite docstrings and comments
EliahKagan Feb 29, 2024
4b04d8a
Better clarify Submodule.branch_path documentation
EliahKagan Feb 29, 2024
254c82a
More docstring revisions within git.refs
EliahKagan Feb 29, 2024
679d2e8
Fix exception type in require_remote_ref_path docstring
EliahKagan Feb 29, 2024
ee0301a
More docstring revisions in second-level modules and git.__init__
EliahKagan Feb 29, 2024
231c3ef
More docstring revisions within git.repo
EliahKagan Feb 29, 2024
e166a0a
More docstring revisions within git.objects
EliahKagan Feb 29, 2024
ffeb7e7
More docstring revisions in git.objects.submodule.base
EliahKagan Feb 29, 2024
ec93955
Further refine some docstring revisions
EliahKagan Feb 29, 2024
63983c2
Remove note in GitCmdObjectDB docstring
EliahKagan Feb 29, 2024
f43292e
Somewhat improve _get_ref_info{,_helper} docstrings
EliahKagan Feb 29, 2024
37c93de
A couple more small docstring refinements
EliahKagan Feb 29, 2024
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
Prev Previous commit
Next Next commit
Revise docstrings in second-level modules
Except for:

- git.cmd, where docstrings were revised in e08066c.

- git.types, where docstring changes may best be made together with
  changes to how imports are organized and documented, which seems
  not to be in the same scope as the changes in this commit.

This change, as well as those in e08066c, are largely along the
lines of #1725, with most revisions here being to docstrings and a
few being to comments.

The major differences between the kinds of docstring changes here
and those ind #1725 are that the changes here push somewhat harder
for consistency and apply some kinds of changes I was reluctant to
apply widely in #1725:

- Wrap all docstrings and comments to 88 columns, except for parts
  that are decisively clearer when not wrapped. Note that semi-
  paragraph changes represented as single newlines are still kept
  where meaningful, which is one reason this is not always the same
  effect as automatic wrapping would produce.

- Avoid code formatting (double backticks) for headings that
  precede sections and code blocks. This was done enough that it
  seems to have been intentional, but it doesn't really have the
  right semantics, and the documentation is currently rendering in
  such a way (including on readthedocs.org) where removing that
  formatting seems clearly better.

- References (single backticks with a role prefix) and code spans
  (double backticks) everywhere applicable, even in the first lines
  of docstrings.

- Single-backticks around parameter names, with no role prefix.
  These were mostly either formatted that way or emphasized (with
  asterisks). This is one of the rare cases that I have used single
  backticks without a role prefix, which ordinarily should be
  avoided, but to get a role for references to a function's
  parameters within that function, a plugin would be needed. In the
  rare case that one function's docstring refers to another
  function's parameters by names those are double-backticked as
  code spans (and where applicable the name of the referred-to
  function is single-backticked with the :func: or :meth: role).

- All sections, such as :param blah:, :note:, and :return:, now
  have a newline before any text in them. This was already often
  but far from always done, and the style was overall inconsistent.
  Of consistent approaches that are clear and easy to write, this
  is the simplest. It also seems to substantially improve
  readability, when taken together with...

- Sections are always separated by a blank line, even if they are
  very short.

- Essentially unlimited use of `~a.b.c`, where applicable, to refer
  and link to the documentation for a.b.c while showing the text
  "a" and revealing "a.b.c" on hover. I had previously somewhat
  limited my use of this tilde notation in case readers of the
  source code itself (where it is not rendered) weren't familiar
  with it, but at the cost of less consistency in when an entity
  was referred to. There remain a couple places in git.util where
  I do not do this because the explicit form `a <a.b.c>`, which is
  equivalent, lined things up better and was thus easier to read.

Those are the major differences between the approach taken here
and in #1725, but not necessarily most of the changes done here
(many of which are the same kinds of revisions as done there).

Note that this commit only modifies some git/*.py files, and there
are more git/**/*.py files that remain to be revised accordingly.
  • Loading branch information
EliahKagan committed Feb 26, 2024
commit 1cd73ba118148e12b5dae7722efd5d86a640f64d
15 changes: 9 additions & 6 deletions git/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,9 @@
:attr:`sys.platform` checks explicitly, especially in cases where it matters which is
used.

:note: ``is_win`` is ``False`` on Cygwin, but is often wrongly assumed ``True``. To
detect Cygwin, use ``sys.platform == "cygwin"``.
:note:
``is_win`` is ``False`` on Cygwin, but is often wrongly assumed ``True``. To detect
Cygwin, use ``sys.platform == "cygwin"``.
"""

is_posix = os.name == "posix"
Expand All @@ -46,9 +47,10 @@
:attr:`sys.platform` checks explicitly, especially in cases where it matters which is
used.

:note: For POSIX systems, more detailed information is available in
:attr:`sys.platform`, while :attr:`os.name` is always ``"posix"`` on such systems,
including macOS (Darwin).
:note:
For POSIX systems, more detailed information is available in :attr:`sys.platform`,
while :attr:`os.name` is always ``"posix"`` on such systems, including macOS
(Darwin).
"""

is_darwin = sys.platform == "darwin"
Expand All @@ -57,7 +59,8 @@
This is deprecated because it clearer to write out :attr:`os.name` or
:attr:`sys.platform` checks explicitly.

:note: For macOS (Darwin), ``os.name == "posix"`` as in other Unix-like systems, while
:note:
For macOS (Darwin), ``os.name == "posix"`` as in other Unix-like systems, while
``sys.platform == "darwin"`.
"""

Expand Down
153 changes: 96 additions & 57 deletions git/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@


class MetaParserBuilder(abc.ABCMeta): # noqa: B024
"""Utility class wrapping base-class methods into decorators that assure read-only properties."""
"""Utility class wrapping base-class methods into decorators that assure read-only
properties."""

def __new__(cls, name: str, bases: Tuple, clsdict: Dict[str, Any]) -> "MetaParserBuilder":
"""Equip all base-class methods with a needs_values decorator, and all non-const
methods with a set_dirty_and_flush_changes decorator in addition to that.
methods with a :func:`set_dirty_and_flush_changes` decorator in addition to
that.
"""
kmm = "_mutating_methods_"
if kmm in clsdict:
Expand All @@ -102,7 +104,8 @@ def __new__(cls, name: str, bases: Tuple, clsdict: Dict[str, Any]) -> "MetaParse


def needs_values(func: Callable[..., _T]) -> Callable[..., _T]:
"""Return a method for ensuring we read values (on demand) before we try to access them."""
"""Return a method for ensuring we read values (on demand) before we try to access
them."""

@wraps(func)
def assure_data_present(self: "GitConfigParser", *args: Any, **kwargs: Any) -> _T:
Expand All @@ -116,7 +119,8 @@ def assure_data_present(self: "GitConfigParser", *args: Any, **kwargs: Any) -> _
def set_dirty_and_flush_changes(non_const_func: Callable[..., _T]) -> Callable[..., _T]:
"""Return a method that checks whether given non constant function may be called.

If so, the instance will be set dirty. Additionally, we flush the changes right to disk.
If so, the instance will be set dirty. Additionally, we flush the changes right to
disk.
"""

def flush_changes(self: "GitConfigParser", *args: Any, **kwargs: Any) -> _T:
Expand All @@ -136,7 +140,8 @@ class SectionConstraint(Generic[T_ConfigParser]):

It supports all ConfigParser methods that operate on an option.

:note: If used as a context manager, will release the wrapped ConfigParser.
:note:
If used as a context manager, will release the wrapped ConfigParser.
"""

__slots__ = ("_config", "_section_name")
Expand Down Expand Up @@ -171,8 +176,8 @@ def __getattr__(self, attr: str) -> Any:
return super().__getattribute__(attr)

def _call_config(self, method: str, *args: Any, **kwargs: Any) -> Any:
"""Call the configuration at the given method which must take a section name
as first argument."""
"""Call the configuration at the given method which must take a section name as
first argument."""
return getattr(self._config, method)(self._section_name, *args, **kwargs)

@property
Expand Down Expand Up @@ -254,7 +259,8 @@ def get_config_path(config_level: Lit_config_levels) -> str:
elif config_level == "repository":
raise ValueError("No repo to get repository configuration from. Use Repo._get_config_path")
else:
# Should not reach here. Will raise ValueError if does. Static typing will warn missing elifs
# Should not reach here. Will raise ValueError if does. Static typing will warn
# about missing elifs.
assert_never( # type: ignore[unreachable]
config_level,
ValueError(f"Invalid configuration level: {config_level!r}"),
Expand All @@ -264,14 +270,15 @@ def get_config_path(config_level: Lit_config_levels) -> str:
class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder):
"""Implements specifics required to read git style configuration files.

This variation behaves much like the git.config command such that the configuration
will be read on demand based on the filepath given during initialization.
This variation behaves much like the ``git config`` command, such that the
configuration will be read on demand based on the filepath given during
initialization.

The changes will automatically be written once the instance goes out of scope, but
can be triggered manually as well.

The configuration file will be locked if you intend to change values preventing other
instances to write concurrently.
The configuration file will be locked if you intend to change values preventing
other instances to write concurrently.

:note:
The config is case-sensitive even when queried, hence section and option names
Expand Down Expand Up @@ -301,7 +308,8 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder):
del optvalueonly_source

_mutating_methods_ = ("add_section", "remove_section", "remove_option", "set")
"""List of RawConfigParser methods able to change the instance."""
"""Names of :class:`~configparser.RawConfigParser` methods able to change the
instance."""

def __init__(
self,
Expand All @@ -311,8 +319,8 @@ def __init__(
config_level: Union[Lit_config_levels, None] = None,
repo: Union["Repo", None] = None,
) -> None:
"""Initialize a configuration reader to read the given file_or_files and to
possibly allow changes to it by setting read_only False.
"""Initialize a configuration reader to read the given `file_or_files` and to
possibly allow changes to it by setting `read_only` False.

:param file_or_files:
A file path or file object, or a sequence of possibly more than one of them.
Expand Down Expand Up @@ -385,7 +393,7 @@ def _acquire_lock(self) -> None:
# END read-only check

def __del__(self) -> None:
"""Write pending changes if required and release locks"""
"""Write pending changes if required and release locks."""
# NOTE: Only consistent in Python 2.
self.release()

Expand All @@ -397,10 +405,12 @@ def __exit__(self, *args: Any) -> None:
self.release()

def release(self) -> None:
"""Flush changes and release the configuration write lock. This instance must not be used anymore afterwards.
"""Flush changes and release the configuration write lock. This instance must
not be used anymore afterwards.

In Python 3, it's required to explicitly release locks and flush changes, as __del__ is not called
deterministically anymore."""
In Python 3, it's required to explicitly release locks and flush changes, as
:meth:`__del__` is not called deterministically anymore.
"""
# Checking for the lock here makes sure we do not raise during write()
# in case an invalid parser was created who could not get a lock.
if self.read_only or (self._lock and not self._lock._has_lock()):
Expand All @@ -424,8 +434,9 @@ def optionxform(self, optionstr: str) -> str:
return optionstr

def _read(self, fp: Union[BufferedReader, IO[bytes]], fpname: str) -> None:
"""Originally a direct copy of the Python 2.4 version of RawConfigParser._read,
to ensure it uses ordered dicts.
"""Originally a direct copy of the Python 2.4 version of
:meth:`RawConfigParser._read <configparser.RawConfigParser._read>`, to ensure it
uses ordered dicts.

The ordering bug was fixed in Python 2.4, and dict itself keeps ordering since
Python 3.7. This has some other changes, especially that it ignores initial
Expand Down Expand Up @@ -525,7 +536,8 @@ def _has_includes(self) -> Union[bool, int]:
def _included_paths(self) -> List[Tuple[str, str]]:
"""List all paths that must be included to configuration.

:return: The list of paths, where each path is a tuple of ``(option, value)``.
:return:
The list of paths, where each path is a tuple of ``(option, value)``.
"""
paths = []

Expand Down Expand Up @@ -577,8 +589,11 @@ def read(self) -> None: # type: ignore[override]
This will ignore files that cannot be read, possibly leaving an empty
configuration.

:return: Nothing
:raise IOError: If a file cannot be handled
:return:
Nothing

:raise IOError:
If a file cannot be handled
"""
if self._is_initialized:
return
Expand All @@ -591,7 +606,7 @@ def read(self) -> None: # type: ignore[override]
elif not isinstance(self._file_or_files, (tuple, list, Sequence)):
# Could merge with above isinstance once runtime type known.
files_to_read = [self._file_or_files]
else: # for lists or tuples
else: # For lists or tuples.
files_to_read = list(self._file_or_files)
# END ensure we have a copy of the paths to handle

Expand All @@ -603,7 +618,8 @@ def read(self) -> None: # type: ignore[override]

if hasattr(file_path, "seek"):
# Must be a file-object.
file_path = cast(IO[bytes], file_path) # TODO: Replace with assert to narrow type, once sure.
# TODO: Replace cast with assert to narrow type, once sure.
file_path = cast(IO[bytes], file_path)
self._read(file_path, file_path.name)
else:
# Assume a path if it is not a file-object.
Expand All @@ -615,8 +631,8 @@ def read(self) -> None: # type: ignore[override]
except IOError:
continue

# Read includes and append those that we didn't handle yet.
# We expect all paths to be normalized and absolute (and will ensure that is the case).
# Read includes and append those that we didn't handle yet. We expect all
# paths to be normalized and absolute (and will ensure that is the case).
if self._has_includes():
for _, include_path in self._included_paths():
if include_path.startswith("~"):
Expand Down Expand Up @@ -695,8 +711,9 @@ def items_all(self, section_name: str) -> List[Tuple[str, List[str]]]:
def write(self) -> None:
"""Write changes to our file, if there are changes at all.

:raise IOError: If this is a read-only writer instance or if we could not obtain
a file lock"""
:raise IOError:
If this is a read-only writer instance or if we could not obtain a file lock
"""
self._assure_writable("write")
if not self._dirty:
return
Expand Down Expand Up @@ -740,7 +757,7 @@ def _assure_writable(self, method_name: str) -> None:
raise IOError("Cannot execute non-constant method %s.%s" % (self, method_name))

def add_section(self, section: str) -> None:
"""Assures added options will stay in order"""
"""Assures added options will stay in order."""
return super().add_section(section)

@property
Expand All @@ -757,16 +774,18 @@ def get_value(
) -> Union[int, float, str, bool]:
"""Get an option's value.

If multiple values are specified for this option in the section, the
last one specified is returned.
If multiple values are specified for this option in the section, the last one
specified is returned.

:param default:
If not None, the given default value will be returned in case
the option did not exist
If not None, the given default value will be returned in case the option did
not exist

:return: a properly typed value, either int, float or string
:return:
A properly typed value, either int, float or string

:raise TypeError: in case the value could not be understood
:raise TypeError:
In case the value could not be understood.
Otherwise the exceptions known to the ConfigParser will be raised.
"""
try:
Expand All @@ -790,12 +809,14 @@ def get_values(
returned.

:param default:
If not None, a list containing the given default value will be
returned in case the option did not exist
If not None, a list containing the given default value will be returned in
case the option did not exist.

:return: a list of properly typed values, either int, float or string
:return:
A list of properly typed values, either int, float or string

:raise TypeError: in case the value could not be understood
:raise TypeError:
In case the value could not be understood.
Otherwise the exceptions known to the ConfigParser will be raised.
"""
try:
Expand Down Expand Up @@ -849,11 +870,17 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo
This will create the section if required, and will not throw as opposed to the
default ConfigParser 'set' method.

:param section: Name of the section in which the option resides or should reside
:param option: Name of the options whose value to set
:param value: Value to set the option to. It must be a string or convertible to
a string.
:return: This instance
:param section:
Name of the section in which the option resides or should reside.

:param option:
Name of the options whose value to set.

:param value:
Value to set the option to. It must be a string or convertible to a string.

:return:
This instance
"""
if not self.has_section(section):
self.add_section(section)
Expand All @@ -865,15 +892,22 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo
def add_value(self, section: str, option: str, value: Union[str, bytes, int, float, bool]) -> "GitConfigParser":
"""Add a value for the given option in section.

This will create the section if required, and will not throw as opposed to the default
ConfigParser 'set' method. The value becomes the new value of the option as returned
by 'get_value', and appends to the list of values returned by 'get_values`'.
This will create the section if required, and will not throw as opposed to the
default ConfigParser 'set' method. The value becomes the new value of the option
as returned by 'get_value', and appends to the list of values returned by
'get_values'.

:param section: Name of the section in which the option resides or should reside
:param option: Name of the option
:param value: Value to add to option. It must be a string or convertible
to a string
:return: This instance
:param section:
Name of the section in which the option resides or should reside.

:param option:
Name of the option.

:param value:
Value to add to option. It must be a string or convertible to a string.

:return:
This instance
"""
if not self.has_section(section):
self.add_section(section)
Expand All @@ -883,8 +917,12 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo
def rename_section(self, section: str, new_name: str) -> "GitConfigParser":
"""Rename the given section to new_name.

:raise ValueError: If ``section`` doesn't exist
:raise ValueError: If a section with ``new_name`` does already exist
:raise ValueError:
If:

* ``section`` doesn't exist.
* A section with ``new_name`` does already exist.

:return: This instance
"""
if not self.has_section(section):
Expand All @@ -898,6 +936,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser":
new_section.setall(k, vs)
# END for each value to copy

# This call writes back the changes, which is why we don't have the respective decorator.
# This call writes back the changes, which is why we don't have the respective
# decorator.
self.remove_section(section)
return self
Loading