-
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.
| indents = ' ' * indent | ||
| if isinstance(node, ast.AST): | ||
| 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)
| offset = self._size + offset | ||
| else: | ||
| 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(args) != type(tuple()): | ||
| raise TypeError("2nd arg must be a tuple") | ||
| 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)
| 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
mainrefactored 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
mainbranch, then run:Help us improve this pull request!