Skip to content

Commit

Permalink
fix number of positional arguments in deprecation warning messages
Browse files Browse the repository at this point in the history
- ignore `cls` and `self`
- use 'arg' for `supported_number=1`
- follow-up f4f2daa, 99b4e62
- see also #215
  • Loading branch information
xflr6 committed Feb 28, 2024
1 parent 5b961eb commit e5578d3
Show file tree
Hide file tree
Showing 7 changed files with 19 additions and 13 deletions.
9 changes: 7 additions & 2 deletions graphviz/_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,16 @@ def promote_pathlike_directory(directory: typing.Union[os.PathLike, str, None],

def deprecate_positional_args(*,
supported_number: int,
ignore_argnames: typing.Sequence[str] = ('cls', 'self'),
category: typing.Type[Warning] = PendingDeprecationWarning,
stacklevel: int = 1):
"""Mark supported_number of positional arguments as the maximum.
Args:
supported_number: Number of positional arguments
for which no warning is raised.
ignore_argnames: Name(s) of arguments to ignore
('cls' and 'self' by default).
category: Type of Warning to raise
or None to return a nulldecorator
returning the undecorated function.
Expand Down Expand Up @@ -143,7 +146,8 @@ def nulldecorator(func):
def decorator(func):
signature = inspect.signature(func)
argnames = [name for name, param in signature.parameters.items()
if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD]
if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD
and name not in ignore_argnames]
log.debug('deprecate positional args: %s.%s(%r)',
func.__module__, func.__qualname__,
argnames[supported_number:])
Expand All @@ -162,7 +166,8 @@ def wrapper(*args, **kwargs):
wanted = ', '.join(f'{name}={value!r}'
for name, value in deprecated.items())
warnings.warn(f'The signature of {func.__name__} will be reduced'
f' to {supported_number} positional args'
f' to {supported_number} positional arg'
f"{'s' if supported_number > 1 else ''}"
f' {list(supported)}: pass {wanted}'
' as keyword arg(s)',
stacklevel=stacklevel,
Expand Down
2 changes: 1 addition & 1 deletion graphviz/saving.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def filepath(self) -> str:
"""The target path for saving the DOT source file."""
return os.path.join(self.directory, self.filename)

@_tools.deprecate_positional_args(supported_number=2)
@_tools.deprecate_positional_args(supported_number=1)
def save(self, filename: typing.Union[os.PathLike, str, None] = None,
directory: typing.Union[os.PathLike, str, None] = None, *,
skip_existing: typing.Optional[bool] = False) -> str:
Expand Down
6 changes: 3 additions & 3 deletions graphviz/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Source(rendering.Render, saving.Save,
"""

@classmethod
@_tools.deprecate_positional_args(supported_number=2)
@_tools.deprecate_positional_args(supported_number=1)
def from_file(cls, filename: typing.Union[os.PathLike, str],
directory: typing.Union[os.PathLike, str, None] = None,
format: typing.Optional[str] = None,
Expand Down Expand Up @@ -73,7 +73,7 @@ def from_file(cls, filename: typing.Union[os.PathLike, str],
renderer=renderer, formatter=formatter,
loaded_from_path=filepath)

@_tools.deprecate_positional_args(supported_number=2)
@_tools.deprecate_positional_args(supported_number=1)
def __init__(self, source: str,
filename: typing.Union[os.PathLike, str, None] = None,
directory: typing.Union[os.PathLike, str, None] = None,
Expand Down Expand Up @@ -122,7 +122,7 @@ def source(self) -> str:
source += '\n'
return source

@_tools.deprecate_positional_args(supported_number=2)
@_tools.deprecate_positional_args(supported_number=1)
def save(self, filename: typing.Union[os.PathLike, str, None] = None,
directory: typing.Union[os.PathLike, str, None] = None, *,
skip_existing: typing.Optional[bool] = None) -> str:
Expand Down
2 changes: 1 addition & 1 deletion tests/backend/test_piping.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_pipe_pipe_invalid_data_mocked(mocker, sentinel, mock_run, quiet):
reason='https://gitlab.com/graphviz/graphviz/-/issues/1269'))])
def test_pipe(capsys, engine, format_, renderer, formatter, pattern,
data=b'graph { spam }'):
with pytest.deprecated_call():
with pytest.deprecated_call(match=r'3 positional args'):
out = graphviz.pipe(engine, format_, data,
renderer, formatter).decode('ascii')

Expand Down
2 changes: 1 addition & 1 deletion tests/backend/test_rendering.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def test_render(capsys, tmp_path, engine, format_, renderer, formatter,
assert lpath.write_bytes(data) == len(data) == lpath.stat().st_size
rendered = lpath.with_suffix(f'{lpath.suffix}.{expected_suffix}')

with pytest.deprecated_call():
with pytest.deprecated_call(match=r'3 positional args'):
result = graphviz.render(engine, format_, str(lpath),
renderer, formatter)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_all_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def test_save_mocked(mocker, dot, filename='nonfilename', directory='nondirector
mock_makedirs = mocker.patch('os.makedirs', autospec=True)
mock_open = mocker.patch('builtins.open', mocker.mock_open())

with pytest.deprecated_call():
with pytest.deprecated_call(match=r'1 positional arg\b'):
assert dot.save(filename, directory) == dot.filepath

assert dot.filename == filename
Expand Down
9 changes: 5 additions & 4 deletions tests/test_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,23 @@ def test_filepath(platform, source):


def test_from_file(tmp_path, filename='hello.gv', directory='source_hello',
data='digraph { hello -> world }', encoding='utf-8'):
data='digraph { hello -> world }', encoding='utf-8',
deprecation_match=r'1 positional arg\b'):
lpath = tmp_path / directory
lpath.mkdir()
(lpath / filename).write_text(data, encoding=encoding)

with pytest.deprecated_call():
with pytest.deprecated_call(match=deprecation_match):
source = graphviz.Source.from_file(filename, str(lpath))
assert source.encoding == 'utf-8'

with pytest.deprecated_call():
with pytest.deprecated_call(match=deprecation_match):
source = graphviz.Source.from_file(filename, str(lpath), encoding=None)
assert source.encoding == locale.getpreferredencoding()

renderer = 'xdot'
formatter = 'core'
with pytest.deprecated_call():
with pytest.deprecated_call(match=deprecation_match):
source = graphviz.Source.from_file(filename, str(lpath),
encoding=encoding,
renderer=renderer,
Expand Down

0 comments on commit e5578d3

Please sign in to comment.