-
Notifications
You must be signed in to change notification settings - Fork 2
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 master branch #18
base: master
Are you sure you want to change the base?
Conversation
if not parentNode in openNodes: | ||
if parentNode not in openNodes: |
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 populate
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan
) - Use f-string instead of string concatenation [×3] (
use-fstring-for-concatenation
) - Replace if statement with if expression (
assign-if-exp
)
piece = remote_file.read(1024) | ||
if not piece: | ||
if piece := remote_file.read(1024): | ||
dl.write(piece) | ||
else: | ||
break | ||
dl.write(piece) |
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 download_thread
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
) - Swap if/else branches (
swap-if-else-branches
)
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.
Walrus (:=) requires Python 3.8+
return sg.popup_get_file('Where to save this file?', 'Download {}'.format( | ||
url), default_path=url.split('/')[-1], save_as=True) | ||
return sg.popup_get_file( | ||
'Where to save this file?', | ||
f'Download {url}', | ||
default_path=url.split('/')[-1], | ||
save_as=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 dlPopup
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
sg.popup("We're sorry!", req.url() + ' could not be fetched. Try again later.') | ||
sg.popup("We're sorry!", f'{req.url()} could not be fetched. Try again later.') | ||
else: | ||
dlpath = dlPopup(req.url()) | ||
if not dlpath is None: | ||
window.FindElement('-DOWNLOADS-').update(value='Downloading {}'.format(dlpath)) | ||
if dlpath is not None: | ||
window.FindElement('-DOWNLOADS-').update(value=f'Downloading {dlpath}') |
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 go
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Simplify logical expression using De Morgan identities (
de-morgan
) - Replace call to format with f-string (
use-fstring-for-formatting
)
if x > 1 or x < 1: | ||
return 's' | ||
return '' | ||
return 's' if x > 1 or x < 1 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 plural
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
)
while not choice in choices: | ||
while choice not in choices: |
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 21-84
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan
) - Simplify conditional into switch-like form [×11] (
switch
) - Remove redundant conditional (
remove-redundant-if
) - Move setting of default value for variable into
else
branch (introduce-default-else
) - Merge nested if conditions (
merge-nested-ifs
) - Merge else clause's nested if statement into elif [×2] (
merge-else-if-into-elif
) - Move assignment closer to its usage within a block [×2] (
move-assign-in-block
) - Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
) - Simplify boolean if expression (
boolean-if-exp-identity
) - Replace if statement with if expression (
assign-if-exp
) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
query = '' | ||
if not (self.query == ''): | ||
query = '%09' + self.query | ||
query = f'%09{self.query}' if self.query != '' else '' | ||
hst = self.host | ||
if not self.port == 70: | ||
hst += ':{}'.format(self.port) | ||
return '{}://{}/{}{}{}'.format(protocol, hst, self.type, path, query) | ||
if self.port != 70: | ||
hst += f':{self.port}' | ||
return f'{protocol}://{hst}/{self.type}{path}{query}' |
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 Request.url
refactored with the following changes:
- Move setting of default value for variable into
else
branch (introduce-default-else
) - Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace call to format with f-string [×2] (
use-fstring-for-formatting
) - Simplify logical expression using De Morgan identities [×2] (
de-morgan
) - Replace if statement with if expression (
assign-if-exp
)
matches = re.match(r'^(.)(.*)\t(.*)\t(.*)\t(.*)', line) | ||
if matches: | ||
if matches := re.match(r'^(.)(.*)\t(.*)\t(.*)\t(.*)', 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 parse_menu
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
up = urlparse('gopher://' + url) | ||
up = urlparse(f'gopher://{url}') | ||
|
||
req.path = up.path | ||
if up.query: | ||
req.path += '?{}'.format(up.query) # NOT to be confused with actual gopher queries, which use %09 | ||
# this just combines them back into one string | ||
req.path += f'?{up.query}' | ||
# this just combines them back into one string |
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_url
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
) - Replace call to format with f-string (
use-fstring-for-formatting
)
This removes the following comments ( why? ):
# NOT to be confused with actual gopher queries, which use %09
path = realpath(gophermap_dir + '/' + path) | ||
path = realpath(f'{gophermap_dir}/{path}') |
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_gophermap
refactored with the following changes:
- Use f-string instead of string concatenation [×5] (
use-fstring-for-concatenation
) - Replace if statement with if expression [×2] (
assign-if-exp
) - Merge nested if conditions (
merge-nested-ifs
) - Replace a for append loop with list extend (
for-append-to-extend
)
96f03c1
to
3088bb1
Compare
Branch
master
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
master
branch, then run:Help us improve this pull request!