-
Notifications
You must be signed in to change notification settings - Fork 17
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 #86
Conversation
95d40bb
to
1be658a
Compare
1be658a
to
053b4c9
Compare
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.44%.
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! |
# Teams and Resources | ||
num_resource_a = 2 | ||
num_resource_b = 2 | ||
|
||
for num_dev_teams in N: | ||
print("-> Num dev teams:", num_dev_teams) | ||
# Teams and Resources | ||
num_resource_a = 2 | ||
num_resource_b = 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.
Lines 90-93
refactored with the following changes:
- Hoist statements out of for/while loops (
hoist-statement-from-loop
)
MAX_TASKS_PER_PERIOD = 2 | ||
MAX_TASKS_IN_PROBLEM = 4 | ||
NB_WORKERS = 10 | ||
NB_TASKS_PER_WORKER = 10 | ||
for horizon in range(20, n, step): | ||
MAX_TASKS_PER_PERIOD = 2 | ||
MAX_TASKS_IN_PROBLEM = 4 | ||
NB_WORKERS = 10 | ||
NB_TASKS_PER_WORKER = 10 |
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 89-122
refactored with the following changes:
- Hoist statements out of for/while loops (
hoist-statement-from-loop
) - Convert for loop into list comprehension (
list-comprehension
)
assertions_str = "".join(["%s" % ass for ass in self.assertions]) | ||
assertions_str = "".join("%s" % ass for ass in self.assertions) |
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 _NamedUIDObject.__repr__
refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator
)
applied_vars = [] | ||
for constraint in list_of_optional_constraints: | ||
applied_vars.append(constraint.applied) | ||
applied_vars = [ | ||
constraint.applied for constraint in list_of_optional_constraints | ||
] |
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 ForceApplyNOptionalConstraints.__init__
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
)
y = [] | ||
for x_ in x: | ||
y.append(self.f(x_)) | ||
|
||
y = [self.f(x_) for x_ in 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 _Cost.plot
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
)
tasks_to_return = {} | ||
for task in tasks_not_unavailable: | ||
if tasks_not_unavailable[task].scheduled: | ||
tasks_to_return[task] = tasks_not_unavailable[task] | ||
return tasks_to_return | ||
return { | ||
task: tasks_not_unavailable[task] | ||
for task in tasks_not_unavailable | ||
if tasks_not_unavailable[task].scheduled | ||
} |
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 SchedulingSolution.get_scheduled_tasks
refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if self.problem.delta_time is not None: | ||
if self.problem.start_time is not None: | ||
problem_properties["problem_start_time"] = self.problem.start_time | ||
problem_properties["problem_end_time"] = ( | ||
self.problem.start_time + self.horizon * self.problem.delta_time | ||
) | ||
else: | ||
problem_properties["problem_start_time"] = time(0) | ||
problem_properties["problem_end_time"] = ( | ||
self.horizon * self.problem.delta_time | ||
) | ||
else: | ||
if self.problem.delta_time is None: | ||
problem_properties["problem_start_time"] = None | ||
problem_properties["problem_end_time"] = None | ||
elif self.problem.start_time is not None: | ||
problem_properties["problem_start_time"] = self.problem.start_time | ||
problem_properties["problem_end_time"] = ( | ||
self.problem.start_time + self.horizon * self.problem.delta_time | ||
) | ||
else: | ||
problem_properties["problem_start_time"] = time(0) | ||
problem_properties["problem_end_time"] = ( | ||
self.horizon * self.problem.delta_time | ||
) |
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 SchedulingSolution.to_json_string
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Merge else clause's nested if statement into elif (
merge-else-if-into-elif
)
colors = [] | ||
for _ in range(len(df)): | ||
colors.append("#%02X%02X%02X" % (r(), r(), r())) | ||
|
||
colors = ["#%02X%02X%02X" % (r(), r(), r()) for _ in df] |
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 SchedulingSolution.render_gantt_plotly
refactored with the following changes:
- Replace index in for loop with direct reference (
for-index-replacement
) - Replace unused for index with underscore (
for-index-underscore
) - Convert for loop into list comprehension (
list-comprehension
)
if length == 0: # zero duration tasks, to be visible | ||
bar_dimension = (start - 0.05, 0.1) | ||
else: | ||
bar_dimension = (start, length) | ||
bar_dimension = (start - 0.05, 0.1) if length == 0 else (start, length) |
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 SchedulingSolution.render_gantt_matplotlib.draw_broken_barh_with_text
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
This removes the following comments ( why? ):
# zero duration tasks, to be visible
new_task_solution.end_time = ( | ||
new_task_solution.start_time + new_task_solution.duration_time | ||
) | ||
else: | ||
new_task_solution.start_time = ( | ||
new_task_solution.start * self._problem.delta_time | ||
) | ||
new_task_solution.end_time = ( | ||
new_task_solution.start_time + new_task_solution.duration_time | ||
) | ||
|
||
new_task_solution.end_time = ( | ||
new_task_solution.start_time + new_task_solution.duration_time | ||
) |
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 SchedulingSolver.build_solution
refactored with the following changes:
- Hoist repeated code outside conditional statement (
hoist-statement-from-if
) - Replace if statement with if expression (
assign-if-exp
) - Simplify boolean if expression (
boolean-if-exp-identity
)
This removes the following comments ( why? ):
# should not be scheduled
# if will be set to False if the resource is an alternative worker
# by default, resource_should_be_assigned is set to True
if max_recursion_depth is not None: | ||
if depth > max_recursion_depth: | ||
warnings.warn( | ||
"maximum recursion depth exceeded. There might be a better solution." | ||
) | ||
break | ||
if max_recursion_depth is not None and depth > max_recursion_depth: | ||
warnings.warn( | ||
"maximum recursion depth exceeded. There might be a better solution." | ||
) | ||
break |
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 SchedulingSolver.solve_optimize_incremental
refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if
) - Merge nested if conditions (
merge-nested-ifs
)
if offset > 0: | ||
lower = task_before.end + offset | ||
else: | ||
lower = task_before.end | ||
lower = task_before.end + offset if offset > 0 else task_before.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 TaskPrecedence.__init__
refactored with the following changes:
- Simplify conditional into switch-like form (
switch
) - Replace if statement with if expression (
assign-if-exp
)
sched_vars = [] | ||
for task in list_of_optional_tasks: | ||
sched_vars.append(task.scheduled) | ||
|
||
sched_vars = [task.scheduled for task in list_of_optional_tasks] |
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 ForceScheduleNOptionalTasks.__init__
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
)
[ | ||
solution.indicators[indicator_id] | ||
for indicator_id in solution.indicators | ||
if "FlowTime" in indicator_id | ||
] | ||
solution.indicators[indicator_id] | ||
for indicator_id in solution.indicators | ||
if "FlowTime" in indicator_id |
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 TestIndicator.get_sum_flowtime
refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator
)
solution = solver.solve() | ||
return solution | ||
return solver.solve() |
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 _solve_problem
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
Codecov Report
@@ Coverage Diff @@
## master #86 +/- ##
==========================================
+ Coverage 94.96% 95.11% +0.15%
==========================================
Files 30 30
Lines 3417 3380 -37
==========================================
- Hits 3245 3215 -30
+ Misses 172 165 -7
Continue to review full report at Codecov.
|
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!