-
Notifications
You must be signed in to change notification settings - Fork 0
Sourcery refactored master 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: master
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.
#Traceback (most recent call last): | ||
#File "<pyshell#119>", line 1, in <module> t.hours |
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 15-30
refactored with the following changes:
- Replace datetime.datetime.today() with datetime.datetime.now() (
use-datetime-now-not-today
)
This removes the following comments ( why? ):
#File "<pyshell#119>", line 1, in <module> t.hours
#Traceback (most recent call last):
if n % 3 == 0 and n % 5 == 0: | ||
return 'Fizz Buzz' | ||
if n % 3 == 0: | ||
return 'Fizz' | ||
if n % 5 == 0: | ||
return 'Buzz' | ||
return n | ||
return 'Fizz Buzz' if n % 5 == 0 else 'Fizz' | ||
return 'Buzz' if n % 5 == 0 else 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 fizzbuzz
refactored with the following changes:
- Hoist a repeated condition into a parent condition (
hoist-repeated-if-condition
) - Lift code into else after jump in control flow [×2] (
reintroduce-else
) - Replace if statement with if expression [×2] (
assign-if-exp
)
win = self._validate_guess(guess) | ||
if win: | ||
if win := self._validate_guess(guess): |
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 Game.__call__
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
print("Roll: {}".format(name)) | ||
for k in row.keys(): | ||
print(f"Roll: {name}") | ||
for k in row: | ||
can_defeat = row[k].strip().lower() == 'win' | ||
print(" * {} will defeat {}? {}".format(name, k, can_defeat)) | ||
print(f" * {name} will defeat {k}? {can_defeat}") |
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_roll
refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting
) - Remove unnecessary call to keys() (
remove-dict-keys
)
print("{}. {} F on {}".format(idx+1, d.actual_max_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_max_temp} F on {d.date}") | ||
print() | ||
print("The coldest 5 days:") | ||
days = research.cold_days() | ||
for idx, d in enumerate(days[:5]): | ||
print("{}. {} F on {}".format(idx+1, d.actual_min_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_min_temp} F on {d.date}") | ||
print() | ||
print("The wettest 5 days:") | ||
|
||
days = research.wet_days() | ||
for idx, d in enumerate(days[:5]): | ||
print("{}. {} inches of rain on {}".format(idx+1, d.actual_precipitation, d.date)) | ||
print(f"{idx + 1}. {d.actual_precipitation} inches of rain on {d.date}") |
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 main
refactored with the following changes:
- Replace call to format with f-string [×3] (
use-fstring-for-formatting
)
record = Record( | ||
**row | ||
) | ||
|
||
return record | ||
return Record(**row) |
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_row
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
#Prints just the data associated with the 'name' key. | ||
|
||
|
||
is_flying = [] | ||
for mount in data['mounts']['collected']: | ||
if mount['isFlying']: | ||
is_flying.append(mount) | ||
|
||
is_flying = [ | ||
mount for mount in data['mounts']['collected'] if mount['isFlying'] | ||
] |
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 25-32
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
)
This removes the following comments ( why? ):
#Prints just the data associated with the 'name' key.
movies = [] | ||
for r in results.get('hits'): | ||
movies.append(Movie(**r)) | ||
|
||
return movies | ||
return [Movie(**r) for r in results.get('hits')] |
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_movie_by_title
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
header_list = [] | ||
#Create BeautifulSoup object | ||
soup = bs4.BeautifulSoup(site.text, 'html.parser') | ||
html_header_list = soup.select('.projectHeader') | ||
|
||
for headers in html_header_list: | ||
header_list.append(headers.getText()) | ||
header_list = [headers.getText() for headers in html_header_list] |
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 scrape
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Convert for loop into list comprehension (
list-comprehension
)
print("{}. {} F on {}".format(idx + 1, d.actual_max_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_max_temp} F on {d.date}") | ||
print() | ||
print("The coldest 5 days:") | ||
|
||
for idx, d in enumerate(cold_days[:5]): | ||
print("{}. {} F on {}".format(idx + 1, d.actual_min_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_min_temp} F on {d.date}") | ||
print() | ||
print("The wettest 5 days:") | ||
|
||
for idx, d in enumerate(wet_days[:5]): | ||
print("{}. {} inches of rain on {}".format(idx + 1, d.actual_precipitation, d.date)) | ||
print(f"{idx + 1}. {d.actual_precipitation} inches of rain on {d.date}") |
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 main
refactored with the following changes:
- Replace call to format with f-string [×3] (
use-fstring-for-formatting
)
print("{}. {} F on {}".format(idx + 1, d.actual_max_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_max_temp} F on {d.date}") | ||
print() | ||
print("The coldest 5 days:") | ||
|
||
for idx, d in enumerate(cold_days[:5]): | ||
print("{}. {} F on {}".format(idx + 1, d.actual_min_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_min_temp} F on {d.date}") | ||
print() | ||
print("The wettest 5 days:") | ||
|
||
for idx, d in enumerate(wet_days[:5]): | ||
print("{}. {} inches of rain on {}".format(idx + 1, d.actual_precipitation, d.date)) | ||
print(f"{idx + 1}. {d.actual_precipitation} inches of rain on {d.date}") |
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 main
refactored with the following changes:
- Replace call to format with f-string [×3] (
use-fstring-for-formatting
)
record = Record( | ||
return Record( | ||
date=row.get('date'), | ||
actual_min_temp=row.get('actual_min_temp'), | ||
actual_max_temp=row.get('actual_max_temp'), | ||
actual_precipitation=row.get('actual_precipitation'), | ||
) | ||
|
||
return record |
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_row
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
print("{}. {} F on {}".format(idx+1, d.actual_max_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_max_temp} F on {d.date}") | ||
print() | ||
print("The coldest 5 days:") | ||
days = research.cold_days() | ||
for idx, d in enumerate(days[:5]): | ||
print("{}. {} F on {}".format(idx+1, d.actual_min_temp, d.date)) | ||
print(f"{idx + 1}. {d.actual_min_temp} F on {d.date}") | ||
print() | ||
print("The wettest 5 days:") | ||
|
||
days = research.wet_days() | ||
for idx, d in enumerate(days[:5]): | ||
print("{}. {} inches of rain on {}".format(idx+1, d.actual_precipitation, d.date)) | ||
print(f"{idx + 1}. {d.actual_precipitation} inches of rain on {d.date}") |
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 main
refactored with the following changes:
- Replace call to format with f-string [×3] (
use-fstring-for-formatting
)
record = Record( | ||
**row | ||
) | ||
|
||
return record | ||
return Record(**row) |
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_row
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
print('%s.db has been created' % name) | ||
print(f'{name}.db has been created') |
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 26-26
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
info = [] | ||
name = input('Enter a name: ') | ||
address = input('Enter an address: ') | ||
number = input('Enter a phone number: ') | ||
for i in (name, address, number): | ||
info.append(i) | ||
|
||
info = [name, address, number] | ||
with sqlite3.connect("addressbook.db") as connection: | ||
c = connection.cursor() | ||
c.execute("INSERT INTO Details VALUES(?, ?, ?)", info) | ||
print('Data inserted to database.\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 enter_details
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Replace identity comprehension with call to collection constructor (
identity-comprehension
) - Convert for loop into list comprehension (
list-comprehension
) - Unwrap a constant iterable constructor (
unwrap-iterable-construction
)
def button_save_click (self, **event_args): | ||
errors = self.validate() | ||
if errors: | ||
def button_save_click(self, **event_args): | ||
if errors := self.validate(): | ||
self.label_errors.text = "\n".join(errors) | ||
return | ||
|
||
name = self.text_box_doc_name.text.strip() | ||
category = self.drop_down_categories.selected_value | ||
contents = self.text_area_contents.text.strip() | ||
|
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 AddDocForm.button_save_click
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
return "{} {} {}".format(d["name"], d["contents"], d["category"]["name"]) | ||
return f'{d["name"]} {d["contents"]} {d["category"]["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 AllDocsForm.doc_to_text
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
results = list(app_tables.documents.search(tables.order_by("created", ascending=False))) | ||
return results | ||
return list( | ||
app_tables.documents.search(tables.order_by("created", ascending=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 all_docs
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
print("Server: Creating new document: {} {} {} {}".format(doc_name, category_name, views, contents, now)) | ||
print( | ||
f"Server: Creating new document: {doc_name} {category_name} {views} {contents}" | ||
) |
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 add_doc
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
cursor = conn.cursor() | ||
yield cursor | ||
yield conn.cursor() |
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 access_db
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
menu = {} | ||
menu['1'] = "Add Room." | ||
menu['2'] = "Add Inventory." | ||
menu['3'] = "View Inventory List." | ||
menu['4'] = "Total Value." | ||
menu['5'] = "Exit." | ||
menu = { | ||
'1': "Add Room.", | ||
'2': "Add Inventory.", | ||
'3': "View Inventory List.", | ||
'4': "Total Value.", | ||
'5': "Exit.", | ||
} |
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 main_menu
refactored with the following changes:
- Merge dictionary assignment with declaration [×5] (
merge-dict-assign
)
cursor.execute("CREATE TABLE '" + name.lower() + "' """" | ||
cursor.execute( | ||
( | ||
f"CREATE TABLE '{name.lower()}" + "' " | ||
""" | ||
(Item TEXT, Value REAL) | ||
""") | ||
""" | ||
) | ||
) |
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 add_room
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
for room in cursor: | ||
room_list.append(room[0]) | ||
room_list.extend(room[0] for room in cursor) |
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 list_rooms
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
)
print("{}. {}".format(idx + 1, r.name)) | ||
print(f"{idx + 1}. {r.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 get_roll_choice
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
if roll1_wins: | ||
return Decision.win | ||
else: | ||
return Decision.lose | ||
return Decision.win if roll1_wins else Decision.lose |
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 decide
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
for k in row.keys(): | ||
for k in row: |
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 __build_roll
refactored with the following changes:
- Remove unnecessary call to keys() (
remove-dict-keys
)
roll = Roll(name) | ||
# TODO: Save | ||
return roll | ||
return Roll(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 create_roll
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
This removes the following comments ( why? ):
# TODO: Save
for row in reader: | ||
rolls.append(build_roll(row)) | ||
|
||
rolls.extend(build_roll(row) for row in reader) |
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 build_rolls
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
)
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!