-
Notifications
You must be signed in to change notification settings - Fork 0
last update (Sourcery refactored) #3
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.
|
|
||
| def __repr__(self): | ||
| return '<{}>'.format(getattr(self, '__name__', self.__class__.__name__)) | ||
| return f"<{getattr(self, '__name__', self.__class__.__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 Thing.__repr__ refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| def run(self, steps=1000): | ||
| """Run the Environment for given number of time steps.""" | ||
| for step in range(steps): | ||
| for _ in range(steps): |
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 Environment.run refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| print(" Thing to be removed: {} at {}".format(thing, thing.location)) | ||
| print(" from list: {}".format([(thing, thing.location) for thing in self.things])) | ||
| print(f" Thing to be removed: {thing} at {thing.location}") | ||
| print(f" from list: {[(thing, thing.location) for thing in self.things]}") |
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 Environment.delete_thing refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
| things = [thing for thing in self.list_things_at(agent.location) if agent.can_grab(thing)] | ||
| if things: | ||
| if things := [ | ||
| thing | ||
| for thing in self.list_things_at(agent.location) | ||
| if agent.can_grab(thing) | ||
| ]: |
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 XYEnvironment.execute_action refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| return not (x < self.x_start or x > self.x_end or y < self.y_start or y > self.y_end) | ||
| return ( | ||
| x >= self.x_start | ||
| and x <= self.x_end | ||
| and y >= self.y_start | ||
| and y <= self.y_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 XYEnvironment.is_inbounds refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| return not (x < self.x_start or x > self.x_end or y < self.y_start or y > self.y_end) | ||
| return ( | ||
| x >= self.x_start | ||
| and x <= self.x_end | ||
| and y >= self.y_start | ||
| and y <= self.y_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 XYEnvironment.is_inbounds refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| row = [] | ||
| for y in range(y_start, y_end): | ||
| row.append(self.list_things_at((x, y))) | ||
| row = [self.list_things_at((x, y)) for y in range(y_start, y_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 GraphicEnvironment.get_world refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension)
| for step in range(steps): | ||
| for _ in range(steps): |
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 GraphicEnvironment.run refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| for x in range(0, len(world)): | ||
| for y in range(0, len(world[x])): | ||
| for x in range(len(world)): | ||
| for y in range(len(world[x])): |
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 GraphicEnvironment.draw_world refactored with the following changes:
- Replace range(0, x) with range(x) [×2] (
remove-zero-from-range)
| row = [] | ||
| for y in range(y_start, y_end): | ||
| row.append(self.list_things_at((x, y))) | ||
| row = [self.list_things_at((x, y)) for y in range(y_start, y_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 WumpusEnvironment.get_world refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension)
| result = [] | ||
| result.append(self.percepts_from(agent, (x - 1, y))) | ||
| result = [self.percepts_from(agent, (x - 1, y))] |
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 WumpusEnvironment.percept refactored with the following changes:
- Merge append into list declaration (
merge-list-append)
| if len(things): | ||
| agent.holding.append(things[0]) | ||
| if len(things): | ||
| agent.holding.append(things[0]) |
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 WumpusEnvironment.execute_action refactored with the following changes:
- Hoist conditional out of nested conditional (
hoist-if-from-if)
| print("Death by {} [-1000].".format(explorer[0].killed_by)) | ||
| print(f"Death by {explorer[0].killed_by} [-1000].") | ||
| else: | ||
| print("Explorer climbed out {}." | ||
| .format("with Gold [+1000]!" if Gold() not in self.things else "without Gold [+0]")) | ||
| print( | ||
| f'Explorer climbed out {"with Gold [+1000]!" if Gold() not in self.things else "without Gold [+0]"}.' | ||
| ) | ||
|
|
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 WumpusEnvironment.is_done refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
| envs = [EnvFactory() for i in range(n)] | ||
| envs = [EnvFactory() for _ in range(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 compare_agents refactored with the following changes:
- Replace unused for index with underscore (
for-index-underscore)
| else: | ||
| assignment = dict(state) | ||
| var = first([v for v in self.variables if v not in assignment]) | ||
| return [(var, val) for val in self.domains[var] | ||
| if self.nconflicts(var, val, assignment) == 0] | ||
| assignment = dict(state) | ||
| var = first([v for v in self.variables if v not in assignment]) | ||
| return [(var, val) for val in self.domains[var] | ||
| if self.nconflicts(var, val, assignment) == 0] |
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 CSP.actions refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else)
| sumv.__name__ = str(n) + "==sum" | ||
| sumv.__name__ = f"{str(n)}==sum" |
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 sum_constraint refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| isv.__name__ = str(val) + "==" | ||
| isv.__name__ = f"{str(val)}==" |
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 is_constraint refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| nev.__name__ = str(val) + "!=" | ||
| nev.__name__ = f"{str(val)}!=" |
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 ne_constraint refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation)
| return SortedSet(to_do, key=lambda t: 1 / len([var for var in t[1].scope])) | ||
| return SortedSet(to_do, key=lambda t: 1 / len(list(t[1].scope))) |
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 sat_up refactored with the following changes:
- Replace identity comprehension with call to collection constructor (
identity-comprehension)
| if len(other_vars) == 0: | ||
| if not other_vars: |
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 ACSolver.GAC refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison)
| else: | ||
| var = other_vars[ind] | ||
| for val in domains[var]: | ||
| # env = dict_union(env, {var:val}) # no side effects | ||
| env[var] = val | ||
| holds, checks = self.any_holds(domains, const, env, other_vars, ind + 1, checks) | ||
| if holds: | ||
| return True, checks | ||
| return False, checks | ||
| var = other_vars[ind] | ||
| for val in domains[var]: | ||
| # env = dict_union(env, {var:val}) # no side effects | ||
| env[var] = val | ||
| holds, checks = self.any_holds(domains, const, env, other_vars, ind + 1, checks) | ||
| if holds: | ||
| return True, checks | ||
| return False, checks |
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 ACSolver.any_holds refactored with the following changes:
- Remove unnecessary else after guard condition (
remove-unnecessary-else)
| var = first(x for x in self.csp.variables if len(new_domains[x]) > 1) | ||
| if var: | ||
| if var := first( | ||
| x for x in self.csp.variables if len(new_domains[x]) > 1 | ||
| ): | ||
| dom1, dom2 = partition_domain(new_domains[var]) | ||
| new_doms1 = extend(new_domains, var, dom1) | ||
| new_doms2 = extend(new_domains, var, dom2) | ||
| to_do = self.new_to_do(var, None) | ||
| return self.domain_splitting(new_doms1, to_do, arc_heuristic) or \ | ||
| self.domain_splitting(new_doms2, to_do, arc_heuristic) | ||
| self.domain_splitting(new_doms2, to_do, arc_heuristic) |
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 ACSolver.domain_splitting refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
| var = "p" + str(j) + str(i) | ||
| var = f"p{str(j)}{str(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 Crossword.__init__ refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation)
| elif assignment is None: | ||
| puzzle += "[_] " | ||
| else: | ||
| var = "p" + str(j) + str(i) | ||
| if assignment is not None: | ||
| if isinstance(assignment[var], set) and len(assignment[var]) == 1: | ||
| puzzle += "[" + str(first(assignment[var])).upper() + "] " | ||
| elif isinstance(assignment[var], str): | ||
| puzzle += "[" + str(assignment[var]).upper() + "] " | ||
| else: | ||
| puzzle += "[_] " | ||
| var = f"p{str(j)}{str(i)}" | ||
| if isinstance(assignment[var], set) and len(assignment[var]) == 1: | ||
| puzzle += f"[{str(first(assignment[var])).upper()}] " | ||
| elif isinstance(assignment[var], str): | ||
| puzzle += f"[{str(assignment[var]).upper()}] " |
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 Crossword.display refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif) - Move assignments closer to their usage (
move-assign) - Use f-string instead of string concatenation [×6] (
use-fstring-for-concatenation) - Swap if/else branches (
swap-if-else-branches)
| for i, line in enumerate(puzzle): | ||
| for j, element in enumerate(line): | ||
| if element != '_' and element != '*': | ||
| if element not in ['_', '*']: |
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 Kakuro.__init__ refactored with the following changes:
- Use f-string instead of string concatenation [×12] (
use-fstring-for-concatenation) - Convert for loop into dictionary comprehension (
dict-comprehension) - Replace multiple comparisons of same variable with
inoperator (merge-comparisons) - Merge consecutive list appends into a single extend [×2] (
merge-list-appends-into-extend)
| for idx in range(0, 24): | ||
| if board[idx][player] > 0: | ||
| return 0 | ||
| return util[player] | ||
| return next((0 for idx in range(24) if board[idx][player] > 0), util[player]) |
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 Backgammon.compute_utility refactored with the following changes:
- Use the built-in function
nextinstead of a for-loop (use-next) - Replace range(0, x) with range(x) (
remove-zero-from-range)
| sum_range = range(0, 7) if player == 'W' else range(17, 24) | ||
| sum_range = range(7) if player == 'W' else range(17, 24) |
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 Backgammon.checkers_at_home refactored with the following changes:
- Replace range(0, x) with range(x) (
remove-zero-from-range)
| dest_range = range(0, 24) | ||
| dest_range = range(24) | ||
| move1_legal = move2_legal = False | ||
| if dest1 in dest_range: | ||
| if self.is_point_open(player, board[dest1]): | ||
| self.move_checker(board, start[0], steps[0], player) | ||
| move1_legal = True | ||
| else: | ||
| if self.allow_bear_off[player]: | ||
| self.move_checker(board, start[0], steps[0], player) | ||
| move1_legal = True | ||
| elif self.allow_bear_off[player]: | ||
| self.move_checker(board, start[0], steps[0], player) | ||
| move1_legal = True | ||
| if not move1_legal: | ||
| return False | ||
| if dest2 in dest_range: | ||
| if self.is_point_open(player, board[dest2]): | ||
| move2_legal = True | ||
| else: | ||
| if self.allow_bear_off[player]: | ||
| move2_legal = True | ||
| elif self.allow_bear_off[player]: | ||
| move2_legal = 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 Backgammon.is_legal_move refactored with the following changes:
- Replace range(0, x) with range(x) (
remove-zero-from-range) - Merge else clause's nested if statement into elif [×2] (
merge-else-if-into-elif)
| dest_range = range(0, 24) | ||
| dest_range = range(24) |
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 Backgammon.move_checker refactored with the following changes:
- Replace range(0, x) with range(x) (
remove-zero-from-range)
| dice_rolls = list(itertools.combinations_with_replacement([1, 2, 3, 4, 5, 6], 2)) | ||
| return dice_rolls | ||
| return list(itertools.combinations_with_replacement([1, 2, 3, 4, 5, 6], 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 Backgammon.chances 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.27%.
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! |
Pull Request #2 refactored by Sourcery.
Since the original Pull Request was opened as a fork in a contributor's
repository, we are unable to create a Pull Request branching from it.
To incorporate these changes, you can either:
Merge this Pull Request instead of the original, or
Ask your contributor to locally incorporate these commits and push them to
the original Pull Request
Incorporate changes via command line
NOTE: As code is pushed to the original Pull Request, Sourcery will
re-run and update (force-push) this Pull Request with new refactorings as
necessary. If Sourcery finds no refactorings at any point, this Pull Request
will be closed automatically.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Help us improve this pull request!