-
Notifications
You must be signed in to change notification settings - Fork 0
Sourcery refactored main branch #1
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
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.
Sourcery timed out performing refactorings.
Due to GitHub API limits, only the first 60 comments can be shown.
lineno = 'row={}'.format(node.lineno) if hasattr(node, 'lineno') else '' | ||
lineno = f'row={node.lineno}' if hasattr(node, 'lineno') else '' |
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 print_node
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
args = parser.parse_args() | ||
return args | ||
return parser.parse_args() |
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 parse_args
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if name[0] == "_": | ||
return name[1:] + "1" | ||
return name + "2" | ||
return f"{name[1:]}1" if name[0] == "_" else f"{name}2" |
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 name_sort_key
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
) - Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
)
attrs = [] | ||
for attr in dir(typ): | ||
if attr_is_not_inherited(typ, attr): | ||
attrs.append((attr, extra_info(getattr(typ, attr)))) | ||
attrs = [ | ||
(attr, extra_info(getattr(typ, attr))) | ||
for attr in dir(typ) | ||
if attr_is_not_inherited(typ, attr) | ||
] |
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 gen_methods
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
)
modname = modname[:-9] + " (package)" | ||
modname = f"{modname[:-9]} (package)" |
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 scan_modules
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
if len(self) < len(other): | ||
return False | ||
for elem in other: | ||
if elem not in self: | ||
return False | ||
return True | ||
return False if len(self) < len(other) else all(elem in self for elem in other) |
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 Set.__ge__
refactored with the following changes:
- Use any() instead of for loop (
use-any
) - Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Invert any/all to simplify comparisons (
invert-any-all
)
for value in other: | ||
if value in self: | ||
return False | ||
return True | ||
return all(value not in self for value in other) |
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 Set.isdisjoint
refactored with the following changes:
- Use any() instead of for loop (
use-any
) - Invert any/all to simplify comparisons (
invert-any-all
)
def _from_iterable(self, it): | ||
def _from_iterable(cls, it): |
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 KeysView._from_iterable
refactored with the following changes:
- The first argument to class methods should be
cls
(class-method-first-arg-name
)
def _from_iterable(self, it): | ||
def _from_iterable(cls, it): |
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 ItemsView._from_iterable
refactored with the following changes:
- The first argument to class methods should be
cls
(class-method-first-arg-name
)
v = self[i] | ||
yield v | ||
yield self[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 Sequence.__iter__
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
for v in self: | ||
if v is value or v == value: | ||
return True | ||
return False | ||
return any(v is value or v == value for v in self) |
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 Sequence.__contains__
refactored with the following changes:
- Use any() instead of for loop (
use-any
)
REVERSE_IMPORT_MAPPING = dict((v, k) for (k, v) in IMPORT_MAPPING.items()) | ||
REVERSE_IMPORT_MAPPING = {v: k for (k, v) in IMPORT_MAPPING.items()} | ||
assert len(REVERSE_IMPORT_MAPPING) == len(IMPORT_MAPPING) | ||
REVERSE_NAME_MAPPING = dict((v, k) for (k, v) in NAME_MAPPING.items()) | ||
REVERSE_NAME_MAPPING = {v: k for (k, v) in NAME_MAPPING.items()} | ||
assert len(REVERSE_NAME_MAPPING) == len(NAME_MAPPING) | ||
|
||
# Non-mutual mappings. | ||
|
||
IMPORT_MAPPING.update({ | ||
IMPORT_MAPPING |= { |
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 173-232
refactored with the following changes:
- Replace list(), dict() or set() with comprehension [×2] (
collection-builtin-to-comprehension
) - Merge dictionary updates via the union operator [×4] (
dict-assign-update-to-union
)
raise ValueError("Invalid value for whence: {}".format(whence)) | ||
raise ValueError(f"Invalid value for whence: {whence}") |
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 DecompressReader.seek
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
) - Use named expression to simplify assignment and conditional (
use-named-expression
) - Lift code into else after jump in control flow (
reintroduce-else
) - Swap if/else branches (
swap-if-else-branches
)
raise TypeError("expected {}.__fspath__() to return str or bytes, " | ||
"not {}".format(path_type.__name__, | ||
type(path_repr).__name__)) | ||
raise TypeError( | ||
f"expected {path_type.__name__}.__fspath__() to return str or bytes, not {type(path_repr).__name__}" | ||
) |
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 fspath
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
if type(kwargs) != type(dict()): | ||
if type(kwargs) != type({}): |
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 start_new_thread
refactored with the following changes:
- Replace
dict()
with{}
(dict-literal
)
m = _declstringlit_match(rawdata, j) | ||
if m: | ||
if m := _declstringlit_match(rawdata, j): |
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 ParserBase._parse_doctype_entity
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
m = _declname_match(rawdata, i) | ||
if m: | ||
if m := _declname_match(rawdata, i): | ||
s = m.group() | ||
name = s.strip() | ||
if (i + len(s)) == n: | ||
return None, -1 # end of buffer | ||
return name.lower(), m.end() | ||
return (None, -1) if (i + len(s)) == n else (name.lower(), m.end()) |
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 ParserBase._scan_name
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
This removes the following comments ( why? ):
# end of buffer
executable = executable + '.exe' | ||
|
||
if not os.path.isfile(executable): | ||
for p in paths: | ||
f = os.path.join(p, executable) | ||
if os.path.isfile(f): | ||
# the file exists, we have a shot at spawn working | ||
return f | ||
return None | ||
else: | ||
executable = f'{executable}.exe' | ||
|
||
if os.path.isfile(executable): | ||
return executable | ||
for p in paths: | ||
f = os.path.join(p, executable) | ||
if os.path.isfile(f): | ||
# the file exists, we have a shot at spawn working | ||
return f | ||
return 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 _find_executable
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
fp = open("/tmp/_osx_support.%s"%( | ||
os.getpid(),), "w+b") | ||
fp = open(f"/tmp/_osx_support.{os.getpid()}", "w+b") | ||
|
||
with contextlib.closing(fp) as fp: | ||
if capture_stderr: | ||
cmd = "%s >'%s' 2>&1" % (commandstring, fp.name) | ||
cmd = f"{commandstring} >'{fp.name}' 2>&1" | ||
else: | ||
cmd = "%s 2>/dev/null >'%s'" % (commandstring, fp.name) | ||
cmd = f"{commandstring} 2>/dev/null >'{fp.name}'" |
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 _read_output
refactored with the following changes:
- Replace interpolated string formatting with f-string [×3] (
replace-interpolation-with-fstring
)
return (_find_executable(toolname) | ||
or _read_output("/usr/bin/xcrun -find %s" % (toolname,)) | ||
or '' | ||
) | ||
return ( | ||
_find_executable(toolname) | ||
or _read_output(f"/usr/bin/xcrun -find {toolname}") | ||
or '' | ||
) |
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 _find_build_tool
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
_SYSTEM_VERSION = '.'.join(m.group(1).split('.')[:2]) | ||
# else: fall back to the default behaviour | ||
_SYSTEM_VERSION = '.'.join(m[1].split('.')[:2]) | ||
# else: fall back to the default behaviour |
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_system_version
refactored with the following changes:
- Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups
)
osx_version = _get_system_version() | ||
if osx_version: | ||
if osx_version := _get_system_version(): |
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_system_version_tuple
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
contents = _read_output('%s -c -E -v - </dev/null' % (cc,), True) | ||
contents = _read_output(f'{cc} -c -E -v - </dev/null', True) |
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 _default_sysroot
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
return bool(osx_version >= (10, 4)) if osx_version else False | ||
return osx_version >= (10, 4) if osx_version 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 _supports_universal_builds
refactored with the following changes:
- Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
cv_split[0] = cc if cv != 'CXX' else cc + '++' | ||
cv_split[0] = cc if cv != 'CXX' else f'{cc}++' |
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 _find_appropriate_compiler
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
def __subclasscheck__(cls, subclass): | ||
def __subclasscheck__(self, subclass): | ||
"""Override for issubclass(subclass, cls).""" | ||
if not isinstance(subclass, type): | ||
raise TypeError('issubclass() arg 1 must be a class') | ||
# Check cache | ||
if subclass in cls._abc_cache: | ||
if subclass in self._abc_cache: | ||
return True | ||
# Check negative cache; may have to invalidate | ||
if cls._abc_negative_cache_version < ABCMeta._abc_invalidation_counter: | ||
if self._abc_negative_cache_version < ABCMeta._abc_invalidation_counter: | ||
# Invalidate the negative cache | ||
cls._abc_negative_cache = WeakSet() | ||
cls._abc_negative_cache_version = ABCMeta._abc_invalidation_counter | ||
elif subclass in cls._abc_negative_cache: | ||
self._abc_negative_cache = WeakSet() | ||
self._abc_negative_cache_version = ABCMeta._abc_invalidation_counter | ||
elif subclass in self._abc_negative_cache: | ||
return False | ||
# Check the subclass hook | ||
ok = cls.__subclasshook__(subclass) | ||
ok = self.__subclasshook__(subclass) | ||
if ok is not NotImplemented: | ||
assert isinstance(ok, bool) | ||
if ok: | ||
cls._abc_cache.add(subclass) | ||
self._abc_cache.add(subclass) | ||
else: | ||
cls._abc_negative_cache.add(subclass) | ||
self._abc_negative_cache.add(subclass) | ||
return ok | ||
# Check if it's a direct subclass | ||
if cls in getattr(subclass, '__mro__', ()): | ||
cls._abc_cache.add(subclass) | ||
if self in getattr(subclass, '__mro__', ()): | ||
self._abc_cache.add(subclass) | ||
return True | ||
# Check if it's a subclass of a registered class (recursive) | ||
for rcls in cls._abc_registry: | ||
for rcls in self._abc_registry: | ||
if issubclass(subclass, rcls): | ||
cls._abc_cache.add(subclass) | ||
self._abc_cache.add(subclass) | ||
return True | ||
# Check if it's a subclass of a subclass (recursive) | ||
for scls in cls.__subclasses__(): | ||
for scls in self.__subclasses__(): | ||
if issubclass(subclass, scls): | ||
cls._abc_cache.add(subclass) | ||
self._abc_cache.add(subclass) | ||
return True | ||
# No dice; update negative cache | ||
cls._abc_negative_cache.add(subclass) | ||
self._abc_negative_cache.add(subclass) |
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 ABCMeta.__subclasscheck__
refactored with the following changes:
- The first argument to instance methods should be
self
(instance-method-first-arg-name
)
if isinstance(obj, str): | ||
res = obj.encode() | ||
else: | ||
res = bytes(obj) | ||
res = obj.encode() if isinstance(obj, str) else bytes(obj) |
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 readbuffer_encode
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if sys.maxunicode == 65535: | ||
unicode_bytes = 2 | ||
else: | ||
unicode_bytes = 4 | ||
unicode_bytes = 2 if sys.maxunicode == 65535 else 4 |
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 160-163
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
for j in range(unicode_bytes): | ||
for _ in range(unicode_bytes): | ||
b.append(i%256) | ||
i >>= 8 | ||
if sys.byteorder == "big": | ||
b.reverse() | ||
p += b | ||
res = bytes(p) | ||
return res, len(res) | ||
else: | ||
res = "You can do better than this" # XXX make this right | ||
return res, len(res) | ||
|
||
return res, len(res) |
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 unicode_internal_encode
refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore
) - Hoist repeated code outside conditional statement (
hoist-statement-from-if
)
p = [] | ||
i = 0 | ||
if sys.byteorder == "big": | ||
start = unicode_bytes - 1 | ||
stop = -1 | ||
step = -1 | ||
else: | ||
p = [] | ||
i = 0 | ||
if sys.byteorder == "big": | ||
start = unicode_bytes - 1 | ||
stop = -1 | ||
step = -1 | ||
else: | ||
start = 0 | ||
stop = unicode_bytes | ||
step = 1 | ||
while i < len(unistr)-unicode_bytes+1: | ||
t = 0 | ||
h = 0 | ||
for j in range(start, stop, step): | ||
t += ord(unistr[i+j])<<(h*8) | ||
h += 1 | ||
i += unicode_bytes | ||
p += chr(t) | ||
res = ''.join(p) | ||
return res, len(res) | ||
start = 0 | ||
stop = unicode_bytes | ||
step = 1 | ||
while i < len(unistr)-unicode_bytes+1: | ||
t = sum( | ||
ord(unistr[i + j]) << (h * 8) | ||
for h, j in enumerate(range(start, stop, step)) | ||
) | ||
i += unicode_bytes | ||
p += chr(t) | ||
res = ''.join(p) | ||
return res, len(res) |
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 unicode_internal_decode
refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else
) - Convert for loop into call to sum() (
sum-comprehension
) - Replace manual loop counter with call to enumerate (
convert-to-enumerate
)
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!