-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sourcery refactored main branch #1
base: main
Are you sure you want to change the base?
Conversation
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.
Due to GitHub API limits, only the first 60 comments can be shown.
html_short_title = "pytest-%s" % release | ||
html_short_title = f"pytest-{release}" |
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.
Lines 196-196
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
|
||
|
||
|
||
class WarnLogFilter(logging.Filter): | ||
def filter(self, record: logging.LogRecord) -> bool: | ||
"""Ignore warnings about missing include with "only" directive. | ||
|
||
Ref: https://github.com/sphinx-doc/sphinx/issues/2150.""" | ||
if ( | ||
record.msg.startswith('Problems with "include" directive path:') | ||
and "_changelog_towncrier_draft.rst" in record.msg | ||
): | ||
return False | ||
return True | ||
return ( | ||
not record.msg.startswith( | ||
'Problems with "include" directive path:' | ||
) | ||
or "_changelog_towncrier_draft.rst" not in record.msg | ||
) | ||
|
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.
Function configure_logging
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Simplify boolean if expression (
boolean-if-exp-identity
) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
assert 1 in [0, 2, 3, 4, 5] | ||
assert 1 in {0, 2, 3, 4, 5} |
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.
Function TestSpecialisedExplanations.test_in_list
refactored with the following changes:
- Use set when checking membership of a collection of literals (
collection-into-set
)
if namenotexi: # NOQA | ||
pass | ||
pass |
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.
Function TestRaises.test_some_error
refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if
)
This removes the following comments ( why? ):
# NOQA
def setup_class(cls): | ||
cls.classcount += 1 | ||
def setup_class(self): | ||
self.classcount += 1 |
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.
Function TestStateFullThing.setup_class
refactored with the following changes:
- The first argument to instance methods should be
self
(instance-method-first-arg-name
)
tw.line("cachedir: " + str(config.cache._cachedir)) | ||
tw.line(f"cachedir: {str(config.cache._cachedir)}") |
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.
Function cacheshow
refactored with the following changes:
- Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
) - Replace interpolated string formatting with f-string [×2] (
replace-interpolation-with-fstring
)
try: | ||
with contextlib.suppress(ImportError): | ||
import colorama # noqa: F401 | ||
except ImportError: | ||
pass |
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.
Function _colorama_workaround
refactored with the following changes:
- Use
contextlib
'ssuppress
method to silence an error (use-contextlib-suppress
)
if not buffered and mode[0] == "w": | ||
buffering = 0 | ||
else: | ||
buffering = -1 | ||
|
||
buffering = 0 if not buffered and mode[0] == "w" else -1 |
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.
Function _windowsconsoleio_workaround
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
tmpfile = CaptureIO() if not tee else TeeCaptureIO(self._old) | ||
tmpfile = TeeCaptureIO(self._old) if tee else CaptureIO() |
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.
Function SysCaptureBinary.__init__
refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression
)
if not isinstance(other, (CaptureResult, tuple)): | ||
return NotImplemented | ||
return tuple(self) == tuple(other) | ||
return ( | ||
tuple(self) == tuple(other) | ||
if isinstance(other, (CaptureResult, tuple)) | ||
else NotImplemented | ||
) |
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.
Function CaptureResult.__eq__
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
if not isinstance(other, (CaptureResult, tuple)): | ||
return NotImplemented | ||
return tuple(self) < tuple(other) | ||
return ( | ||
tuple(self) < tuple(other) | ||
if isinstance(other, (CaptureResult, tuple)) | ||
else NotImplemented | ||
) |
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.
Function CaptureResult.__lt__
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
return "fixture %s" % self._capture_fixture.request.fixturename | ||
return f"fixture {self._capture_fixture.request.fixturename}" |
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.
Function CaptureManager.is_capturing
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
"cannot use {} and {} at the same time".format( | ||
requested_fixture, current_fixture | ||
) | ||
f"cannot use {requested_fixture} and {current_fixture} at the same time" | ||
) | ||
|
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.
Function CaptureManager.set_fixture
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
if self._capture is not None: | ||
return self._capture.is_started() | ||
return False | ||
return self._capture.is_started() if self._capture is not None else False |
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.
Function CaptureFixture._is_started
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
@@ -1,4 +1,5 @@ | |||
"""Python version compatibility code.""" | |||
|
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.
Lines 23-31
refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if
)
This removes the following comments ( why? ):
# fmt: off
# fmt: on
# If `overload` is imported from `compat` instead of from `typing`,
# Sphinx doesn't recognize it as `overload` and the API docs for
# Workaround for https://github.com/sphinx-doc/sphinx/issues/10351.
# it fine.
# overloaded functions look good again. But type checkers handle
if continue_on_failure: | ||
# We need to turn off this if we use pdb since we should stop at | ||
# the first failure. | ||
if config.getvalue("usepdb"): | ||
continue_on_failure = False | ||
if continue_on_failure and config.getvalue("usepdb"): | ||
continue_on_failure = False |
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.
Function _get_continue_on_failure
refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs
)
This removes the following comments ( why? ):
# We need to turn off this if we use pdb since we should stop at
# the first failure.
return dict() | ||
return {} |
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.
Function doctest_namespace
refactored with the following changes:
- Replace dict() with {} (
dict-literal
)
fixture_package_name = "{}/{}".format(fixturedef.baseid, "__init__.py") | ||
fixture_package_name = f"{fixturedef.baseid}/__init__.py" | ||
while current and ( | ||
type(current) is not cls or fixture_package_name != current.nodeid | ||
): | ||
current = current.parent | ||
if current is None: | ||
return node.session | ||
return current | ||
return node.session if current is None else current |
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.
Function get_scope_package
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
) - Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Simplify unnecessary nesting, casting and constant values in f-strings (
simplify-fstring-formatting
)
keys = dict.fromkeys(get_parametrized_fixture_keys(item, scope), None) | ||
if keys: | ||
if keys := dict.fromkeys( | ||
get_parametrized_fixture_keys(item, scope), None | ||
): |
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.
Function reorder_items
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
argkeys = dict.fromkeys( | ||
(k for k in scoped_argkeys_cache.get(item, []) if k not in ignore), None | ||
) | ||
if not argkeys: | ||
no_argkey_group[item] = None | ||
else: | ||
if argkeys := dict.fromkeys( | ||
( | ||
k | ||
for k in scoped_argkeys_cache.get(item, []) | ||
if k not in ignore | ||
), | ||
None, | ||
): |
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.
Function reorder_items_atscope
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Swap if/else branches (
swap-if-else-branches
)
clscol = self._pyfuncitem.getparent(_pytest.python.Class) | ||
if clscol: | ||
if clscol := self._pyfuncitem.getparent(_pytest.python.Class): |
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.
Function FixtureRequest.cls
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
fixtures_not_supported = getattr(funcitem, "nofuncargs", False) | ||
if has_params and fixtures_not_supported: | ||
msg = ( | ||
"{name} does not support fixtures, maybe unittest.TestCase subclass?\n" | ||
"Node id: {nodeid}\n" | ||
"Function type: {typename}" | ||
).format( | ||
name=funcitem.name, | ||
nodeid=funcitem.nodeid, | ||
typename=type(funcitem).__name__, | ||
) | ||
fail(msg, pytrace=False) | ||
if has_params: | ||
fixtures_not_supported = getattr(funcitem, "nofuncargs", False) | ||
if fixtures_not_supported: | ||
msg = ( | ||
"{name} does not support fixtures, maybe unittest.TestCase subclass?\n" | ||
"Node id: {nodeid}\n" | ||
"Function type: {typename}" | ||
).format( | ||
name=funcitem.name, | ||
nodeid=funcitem.nodeid, | ||
typename=type(funcitem).__name__, | ||
) | ||
fail(msg, pytrace=False) |
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.
Function FixtureRequest._compute_fixture_value
refactored with the following changes:
- Hoist a repeated condition into a parent condition (
hoist-repeated-if-condition
) - Move assignments closer to their usage (
move-assign
) - Replace call to format with f-string (
use-fstring-for-formatting
)
error_msg = "file %s, line %s: source code not available" | ||
addline(error_msg % (fspath, lineno + 1)) | ||
addline(f"file {fspath}, line {lineno + 1}: source code not available") | ||
else: | ||
addline(f"file {fspath}, line {lineno + 1}") | ||
for i, line in enumerate(lines): | ||
for line in lines: | ||
line = line.rstrip() | ||
addline(" " + line) | ||
addline(f" {line}") |
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.
Function FixtureLookupError.formatrepr
refactored with the following changes:
- Inline variable that is only used once (
inline-variable
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Remove unnecessary calls to
enumerate
when the index is not used (remove-unused-enumerate
) - Replace call to format with f-string [×2] (
use-fstring-for-formatting
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
lines = self.errorstring.split("\n") | ||
if lines: | ||
if lines := self.errorstring.split("\n"): |
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.
Function FixtureLookupErrorRepr.toterminal
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
"Error evaluating {} while defining fixture '{}'.\n" | ||
"Expected a function with the signature (*, fixture_name, config)".format( | ||
scope_callable, fixture_name | ||
) | ||
f"Error evaluating {scope_callable} while defining fixture '{fixture_name}'.\nExpected a function with the signature (*, fixture_name, config)" | ||
) from e | ||
|
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.
Function _eval_scope_callable
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
plugininfo = config.pluginmanager.list_plugin_distinfo() | ||
if plugininfo: | ||
if plugininfo := config.pluginmanager.list_plugin_distinfo(): | ||
lines.append("setuptools registered plugins:") | ||
for plugin, dist in plugininfo: | ||
loc = getattr(plugin, "__file__", repr(plugin)) | ||
content = f"{dist.project_name}-{dist.version} at {loc}" | ||
lines.append(" " + content) | ||
lines.append(f" {content}") |
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.
Function getpluginversioninfo
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
verinfo = getpluginversioninfo(config) | ||
if verinfo: | ||
if verinfo := getpluginversioninfo(config): | ||
lines.extend(verinfo) | ||
|
||
if config.option.traceconfig: | ||
lines.append("active plugins:") | ||
items = config.pluginmanager.list_name_plugin() | ||
for name, plugin in items: | ||
if hasattr(plugin, "__file__"): | ||
r = plugin.__file__ | ||
else: | ||
r = repr(plugin) | ||
r = plugin.__file__ if hasattr(plugin, "__file__") else repr(plugin) |
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.
Function pytest_report_header
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace if statement with if expression (
assign-if-exp
)
if i <= 0xFF: | ||
return "#x%02X" % i | ||
else: | ||
return "#x%04X" % i | ||
return "#x%02X" % i if i <= 0xFF else "#x%04X" % i |
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.
Function bin_xml_escape
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
families = {} | ||
families["_base"] = {"testcase": ["classname", "name"]} | ||
families["_base_legacy"] = {"testcase": ["file", "line", "url"]} | ||
families = { | ||
"_base": {"testcase": ["classname", "name"]}, | ||
"_base_legacy": {"testcase": ["file", "line", "url"]}, | ||
} |
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.
Lines 77-79
refactored with the following changes:
- Merge dictionary assignment with declaration [×2] (
merge-dict-assign
)
self.properties.append((str(name), bin_xml_escape(value))) | ||
self.properties.append((name, bin_xml_escape(value))) |
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.
Function _NodeReporter.add_property
refactored with the following changes:
- Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.19%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Branch
main
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
main
branch, then run:Help us improve this pull request!