Sourcery refactored main branch #1
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Comment on lines
-331
to
+333
| "Warning: build in %s is using versioneer.py from %s" | ||
| % (os.path.dirname(me), versioneer_py) | ||
| f"Warning: build in {os.path.dirname(me)} is using versioneer.py from {versioneer_py}" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Function get_root refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-412
to
+425
| print("unable to run %s" % dispcmd) | ||
| print(f"unable to run {dispcmd}") | ||
| print(e) | ||
| return None, None | ||
| else: | ||
| if verbose: | ||
| print("unable to find command, tried %s" % (commands,)) | ||
| print(f"unable to find command, tried {commands}") | ||
| return None, None | ||
| stdout = p.communicate()[0].strip() | ||
| if sys.version_info[0] >= 3: | ||
| stdout = stdout.decode() | ||
| if p.returncode != 0: | ||
| if verbose: | ||
| print("unable to run %s (error)" % dispcmd) | ||
| print("stdout was %s" % stdout) | ||
| print(f"unable to run {dispcmd} (error)") | ||
| print(f"stdout was {stdout}") |
There was a problem hiding this comment.
Function run_command refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-964
to
+974
| f = open(versionfile_abs, "r") | ||
| for line in f.readlines(): | ||
| if line.strip().startswith("git_refnames ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["refnames"] = mo.group(1) | ||
| if line.strip().startswith("git_full ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["full"] = mo.group(1) | ||
| if line.strip().startswith("git_date ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["date"] = mo.group(1) | ||
| f.close() | ||
| with open(versionfile_abs, "r") as f: | ||
| for line in f.readlines(): | ||
| if line.strip().startswith("git_refnames ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["refnames"] = mo[1] | ||
| if line.strip().startswith("git_full ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["full"] = mo[1] | ||
| if line.strip().startswith("git_date ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["date"] = mo[1] |
There was a problem hiding this comment.
Function git_get_keywords refactored with the following changes:
- Use
withwhen opening file to ensure closure (ensure-file-closed) - Use named expression to simplify assignment and conditional (
use-named-expression) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups)
| refs = set([r.strip() for r in refnames.strip("()").split(",")]) | ||
| refs = {r.strip() for r in refnames.strip("()").split(",")} | ||
| # starting in git-1.8.3, tags are listed as "tag: foo-1.0" instead of | ||
| # just "foo-1.0". If we see a "tag: " prefix, prefer those. | ||
| TAG = "tag: " | ||
| tags = set([r[len(TAG) :] for r in refs if r.startswith(TAG)]) | ||
| tags = {r[len(TAG) :] for r in refs if r.startswith(TAG)} |
There was a problem hiding this comment.
Function git_versions_from_keywords refactored with the following changes:
- Replace list(), dict() or set() with comprehension (
collection-builtin-to-comprehension) - Replace unneeded comprehension with generator (
comprehension-to-generator) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
| GITS = ["git"] | ||
| if sys.platform == "win32": | ||
| GITS = ["git.cmd", "git.exe"] | ||
|
|
||
| GITS = ["git.cmd", "git.exe"] if sys.platform == "win32" else ["git"] | ||
| out, rc = run_command(GITS, ["rev-parse", "--git-dir"], cwd=root, hide_stderr=True) | ||
| if rc != 0: | ||
| if verbose: | ||
| print("Directory %s not under git control" % root) | ||
| print(f"Directory {root} not under git control") |
There was a problem hiding this comment.
Function git_pieces_from_vcs refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp) - Move setting of default value for variable into
elsebranch (introduce-default-else) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring) - Merge dictionary assignment with declaration (
merge-dict-assign) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups)
This removes the following comments ( why? ):
# maybe improved later
Comment on lines
-1623
to
+1603
| print("UPDATING %s" % target_versionfile) | ||
| print(f"UPDATING {target_versionfile}") |
There was a problem hiding this comment.
Function get_cmdclass.cmd_build_exe.run refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-1656
to
+1636
| print("UPDATING %s" % target_versionfile) | ||
| print(f"UPDATING {target_versionfile}") |
There was a problem hiding this comment.
Function get_cmdclass.cmd_py2exe.run refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-1699
to
+1679
| print("UPDATING %s" % target_versionfile) | ||
| print(f"UPDATING {target_versionfile}") |
There was a problem hiding this comment.
Function get_cmdclass.cmd_sdist.make_release_tree refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-1770
to
+1750
| print(" creating %s" % cfg.versionfile_source) | ||
| print(f" creating {cfg.versionfile_source}") |
There was a problem hiding this comment.
Function do_setup refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
| value_to_append = "{}={}".format(values, os.environ[values]) | ||
| value_to_append = f"{values}={os.environ[values]}" |
There was a problem hiding this comment.
Function MimicDockerEnvHandling.__call__ refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-303
to
+309
| 'Cannot mount "{}" in editable mode ' | ||
| "as it is not a directory".format(args.repo), | ||
| f'Cannot mount "{args.repo}" in editable mode as it is not a directory', | ||
| extra=dict(phase="failed"), | ||
| ) | ||
| sys.exit(1) | ||
|
|
||
| if args.image_name: | ||
| r2d.output_image_spec = args.image_name | ||
| else: | ||
| # we will pick a name after fetching the repository | ||
| r2d.output_image_spec = "" | ||
| sys.exit(1) | ||
|
|
||
| r2d.output_image_spec = args.image_name or "" |
There was a problem hiding this comment.
Function make_r2d refactored with the following changes:
- Simplify if expression by using or (
or-if-exp-identity) - Replace call to format with f-string. (
use-fstring-for-formatting) - Replace if statement with if expression (
assign-if-exp)
This removes the following comments ( why? ):
# we will pick a name after fetching the repository
| keywords = {"refnames": git_refnames, "full": git_full, "date": git_date} | ||
| return keywords | ||
| return {"refnames": git_refnames, "full": git_full, "date": git_date} |
There was a problem hiding this comment.
Function get_keywords refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
Comment on lines
-92
to
+104
| print("unable to run %s" % dispcmd) | ||
| print(f"unable to run {dispcmd}") | ||
| print(e) | ||
| return None, None | ||
| else: | ||
| if verbose: | ||
| print("unable to find command, tried %s" % (commands,)) | ||
| print(f"unable to find command, tried {commands}") | ||
| return None, None | ||
| stdout = p.communicate()[0].strip() | ||
| if sys.version_info[0] >= 3: | ||
| stdout = stdout.decode() | ||
| if p.returncode != 0: | ||
| if verbose: | ||
| print("unable to run %s (error)" % dispcmd) | ||
| print("stdout was %s" % stdout) | ||
| print(f"unable to run {dispcmd} (error)") | ||
| print(f"stdout was {stdout}") |
There was a problem hiding this comment.
Function run_command refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
Comment on lines
-119
to
+118
| for i in range(3): | ||
| for _ in range(3): |
There was a problem hiding this comment.
Function versions_from_parentdir refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore) - Remove unnecessary else after guard condition (
remove-unnecessary-else) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring) - Remove unnecessary calls to
str()from formatted values in f-strings (remove-str-from-fstring)
Comment on lines
-150
to
+158
| f = open(versionfile_abs, "r") | ||
| for line in f.readlines(): | ||
| if line.strip().startswith("git_refnames ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["refnames"] = mo.group(1) | ||
| if line.strip().startswith("git_full ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["full"] = mo.group(1) | ||
| if line.strip().startswith("git_date ="): | ||
| mo = re.search(r'=\s*"(.*)"', line) | ||
| if mo: | ||
| keywords["date"] = mo.group(1) | ||
| f.close() | ||
| with open(versionfile_abs, "r") as f: | ||
| for line in f.readlines(): | ||
| if line.strip().startswith("git_refnames ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["refnames"] = mo[1] | ||
| if line.strip().startswith("git_full ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["full"] = mo[1] | ||
| if line.strip().startswith("git_date ="): | ||
| if mo := re.search(r'=\s*"(.*)"', line): | ||
| keywords["date"] = mo[1] |
There was a problem hiding this comment.
Function git_get_keywords refactored with the following changes:
- Use
withwhen opening file to ensure closure (ensure-file-closed) - Use named expression to simplify assignment and conditional (
use-named-expression) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups)
Comment on lines
-706
to
+707
| if tag == self.output_image_spec + ":latest": | ||
| if tag == f'{self.output_image_spec}:latest': |
There was a problem hiding this comment.
Function Repo2Docker.find_image refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
Comment on lines
+730
to
+742
| elif self.git_workdir is None: | ||
| checkout_path = tempfile.mkdtemp(prefix="repo2docker") | ||
| else: | ||
| if self.git_workdir is None: | ||
| checkout_path = tempfile.mkdtemp(prefix="repo2docker") | ||
| else: | ||
| checkout_path = self.git_workdir | ||
| checkout_path = self.git_workdir | ||
|
|
||
| try: | ||
| self.fetch(self.repo, self.ref, checkout_path) | ||
|
|
||
| if self.find_image(): | ||
| self.log.info( | ||
| "Reusing existing image ({}), not " | ||
| "building.".format(self.output_image_spec) | ||
| f"Reusing existing image ({self.output_image_spec}), not building." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Function Repo2Docker.build refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif) - Replace call to format with f-string. (
use-fstring-for-formatting) - Merge dictionary updates via the union operator. (
dict-assign-update-to-union)
| return "Image(tags={},config={})".format(self.tags, self.config) | ||
| return f"Image(tags={self.tags},config={self.config})" |
There was a problem hiding this comment.
Function Image.__repr__ refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-23
to
+30
| for vstr in reversed(versions_list): | ||
| if matcher.match(str_to_version(vstr)): | ||
| return vstr | ||
| return None | ||
| return next( | ||
| ( | ||
| vstr | ||
| for vstr in reversed(versions_list) | ||
| if matcher.match(str_to_version(vstr)) | ||
| ), | ||
| None, | ||
| ) |
There was a problem hiding this comment.
Function find_semver_match refactored with the following changes:
- Use the built-in function
nextinstead of a for-loop (use-next)
Comment on lines
-31
to
+35
| return tuple([int(n) for n in vstr.split(".")]) | ||
| return tuple(int(n) for n in vstr.split(".")) |
There was a problem hiding this comment.
Function str_to_version refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator)
Comment on lines
-79
to
+101
| comparison_symbol = constraint_str[0 : first_digit.start()].strip() | ||
| comparison_symbol = constraint_str[:first_digit.start()].strip() | ||
|
|
||
| # Default to "^" search if no matching mode specified (up to next major version) | ||
| if (first_digit.start() == 0) or (comparison_symbol == "^"): | ||
| if major(constraint) == 0: | ||
| if major(constraint) != 0: | ||
| return VersionRange(constraint, (major(constraint) + 1,), True) | ||
|
|
||
| # Also, julia treats pre-1.0 releases specially, as if the first | ||
| # non-zero number is actually a major number: | ||
| # https://docs.julialang.org/en/latest/stdlib/Pkg/#Caret-specifiers-1 | ||
| # So we need to handle it separately by bumping the first non-zero | ||
| # enumber. | ||
| for i, n in enumerate(constraint): | ||
| if ( | ||
| for i, n in enumerate(constraint): | ||
| if ( | ||
| n != 0 or i == len(constraint) - 1 | ||
| ): # (using the last existing number handles situations like "^0.0" or "^0") | ||
| upper = constraint[0:i] + (n + 1,) | ||
| break | ||
| return VersionRange(constraint, upper, True) | ||
| else: | ||
| return VersionRange(constraint, (major(constraint) + 1,), True) | ||
|
|
||
| upper = constraint[:i] + (n + 1,) | ||
| break | ||
| return VersionRange(constraint, upper, True) |
There was a problem hiding this comment.
Function create_semver_matcher refactored with the following changes:
- Replace a[0:x] with a[:x] and a[x:len(a)] with a[x:] (
remove-redundant-slice-index) - Swap if/else branches (
swap-if-else-branches) - Remove unnecessary else after guard condition (
remove-unnecessary-else)
Comment on lines
-123
to
+128
| raise ValueError( | ||
| 'Port specification "{}" has ' "an invalid port.".format(mapping) | ||
| ) | ||
| raise ValueError(f'Port specification "{mapping}" has an invalid port.') | ||
| if p > 65535: | ||
| raise ValueError( | ||
| 'Port specification "{}" specifies ' | ||
| "a port above 65535.".format(mapping) | ||
| f'Port specification "{mapping}" specifies a port above 65535.' | ||
| ) | ||
|
|
There was a problem hiding this comment.
Function validate_and_generate_port_mapping.check_port refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-138
to
+136
| raise ValueError( | ||
| 'Port specification "{}" has ' | ||
| "an invalid protocol.".format(mapping) | ||
| ) | ||
| raise ValueError(f'Port specification "{mapping}" has an invalid protocol.') |
There was a problem hiding this comment.
Function validate_and_generate_port_mapping.check_port_string refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-302
to
+297
| return int(float(num) * self.UNIT_SUFFIXES[suffix]) | ||
| return int(num * self.UNIT_SUFFIXES[suffix]) |
There was a problem hiding this comment.
Function ByteSpecification.validate refactored with the following changes:
- Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast)
Comment on lines
-368
to
+363
| if ignore is not None: | ||
| ignored_names = ignore(src, names) | ||
| else: | ||
| ignored_names = set() | ||
|
|
||
| ignored_names = ignore(src, names) if ignore is not None else set() |
There was a problem hiding this comment.
Function copytree refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Comment on lines
-217
to
+213
| url = "https://mran.microsoft.com/snapshot/{}".format(try_date.isoformat()) | ||
| url = f"https://mran.microsoft.com/snapshot/{try_date.isoformat()}" | ||
| r = requests.head(url) | ||
| if r.ok: | ||
| return url | ||
| raise ValueError( | ||
| "No snapshot found for {} or {} days prior in mran.microsoft.com".format( | ||
| snapshot_date.strftime("%Y-%m-%d"), max_days_prior | ||
| ) | ||
| f'No snapshot found for {snapshot_date.strftime("%Y-%m-%d")} or {max_days_prior} days prior in mran.microsoft.com' |
There was a problem hiding this comment.
Function RBuildPack.get_mran_snapshot_url refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
Comment on lines
-273
to
+262
| if V(self.r_version) >= V("4"): | ||
| vs = "40" | ||
| else: | ||
| vs = "35" | ||
|
|
||
| vs = "40" if V(self.r_version) >= V("4") else "35" |
There was a problem hiding this comment.
Function RBuildPack.get_build_scripts refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp)
Comment on lines
-390
to
+379
| # Delete /tmp/downloaded_packages only if install.R fails, as the second | ||
| # invocation of install.R might be able to reuse them | ||
| "Rscript %s && touch /tmp/.preassembled || true && rm -rf /tmp/downloaded_packages" | ||
| % installR_path, | ||
| f"Rscript {installR_path} && touch /tmp/.preassembled || true && rm -rf /tmp/downloaded_packages", | ||
| ) | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
Function RBuildPack.get_preassemble_scripts refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring)
This removes the following comments ( why? ):
# invocation of install.R might be able to reuse them
# Delete /tmp/downloaded_packages only if install.R fails, as the second
Comment on lines
-408
to
394
| # only run install.R if the pre-assembly failed | ||
| # Delete any downloaded packages in /tmp, as they aren't reused by R | ||
| """if [ ! -f /tmp/.preassembled ]; then Rscript {}; rm -rf /tmp/downloaded_packages; fi""".format( | ||
| installR_path | ||
| ), | ||
| f"""if [ ! -f /tmp/.preassembled ]; then Rscript {installR_path}; rm -rf /tmp/downloaded_packages; fi""", | ||
| ) | ||
| ] | ||
|
|
There was a problem hiding this comment.
Function RBuildPack.get_assemble_scripts refactored with the following changes:
- Replace call to format with f-string. (
use-fstring-for-formatting)
This removes the following comments ( why? ):
# only run install.R if the pre-assembly failed
# Delete any downloaded packages in /tmp, as they aren't reused by R
| env = super().get_env() + [("CONDA_DEFAULT_ENV", "${KERNEL_PYTHON_PREFIX}")] | ||
| return env | ||
| return super().get_env() + [("CONDA_DEFAULT_ENV", "${KERNEL_PYTHON_PREFIX}")] |
There was a problem hiding this comment.
Function CondaBuildPack.get_env refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable)
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.40%.
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! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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!