-
Notifications
You must be signed in to change notification settings - Fork 0
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 #2
base: master
Are you sure you want to change the base?
Conversation
for l in f: | ||
if not l.startswith("#"): | ||
install_requires.append(l.split("#", 1)[0].strip()) | ||
|
||
install_requires.extend( | ||
l.split("#", 1)[0].strip() for l in f if not l.startswith("#") | ||
) |
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 8-11
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
)
if id is None: | ||
return url | ||
|
||
return str(URL(url).with_query({"client_id": id})) | ||
return url if id is None else str(URL(url).with_query({"client_id": 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 build_request_url
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
)
if cilogon_client_id != stored_client_id: | ||
return False | ||
|
||
return True | ||
return cilogon_client_id == stored_client_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 stored_client_id_same_with_cilogon_records
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
) - Simplify boolean if expression (
boolean-if-exp-identity
) - Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
client_id = load_client_id_from_file(config_filename) | ||
if client_id: | ||
if client_id := load_client_id_from_file(config_filename): |
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_client
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
if not client_id: | ||
if Path(config_filename).is_file(): | ||
client_id = load_client_id_from_file(config_filename) | ||
# Nothing to do if no client has been found | ||
if not client_id: | ||
print_colour( | ||
"No `client_id` to delete was provided and couldn't find any in `config_filename`", | ||
"red", | ||
) | ||
return | ||
else: | ||
print_colour( | ||
f"No `client_id` to delete was provided and couldn't find any {config_filename} file", | ||
"red", | ||
) | ||
return | ||
else: | ||
if client_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 delete_client
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
)
self.cluster.config_path.joinpath(p) | ||
for p in self.spec["helm_chart_values_files"] | ||
) as values_files: | ||
self.cluster.config_path.joinpath(p) | ||
for p in self.spec["helm_chart_values_files"] | ||
) as values_files: |
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 Hub.deploy
refactored with the following changes:
- Replace a for append loop with list extend (
for-append-to-extend
)
# A dictionary to convert row values when they are Boolean | ||
boolean_converter = { | ||
True: "Yes", | ||
False: "No", | ||
} | ||
|
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_markdown_comment
refactored with the following changes:
- Move assignments closer to their usage (
move-assign
)
query = f""" | ||
return f""" |
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_gcp_query
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
rows = p.query_range(prom_query, start, end, step) | ||
return rows | ||
return p.query_range(prom_query, start, end, step) |
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 PrometheusUtilizationImporter._run_query
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
df["support_combined"] = df["support_combined"] + df["support"] | ||
df["support_combined"] += df["support"] |
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 PrometheusUtilizationImporter.combine_support
refactored with the following changes:
- Replace assignment with augmented assignment (
aug-assign
)
result = bq_importer.get_costs(start_month, end_month) | ||
return result | ||
return bq_importer.get_costs(start_month, end_month) |
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_dedicated_cluster_costs
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
"username": "".join(secrets.choice(alphabet) for i in range(64)), | ||
"password": "".join(secrets.choice(alphabet) for i in range(64)), | ||
"username": "".join(secrets.choice(alphabet) for _ in range(64)), | ||
"password": "".join(secrets.choice(alphabet) for _ in range(64)), |
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 generate_support_files
refactored with the following changes:
- Replace unused for index with underscore [×2] (
for-index-underscore
)
datasource_details = { | ||
return { |
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_datasource_details
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
headers = { | ||
return { | ||
"Accept": "application/json", | ||
"Content-Type": "application/json", | ||
"Authorization": f"Bearer {token}", | ||
} | ||
|
||
return headers |
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_datasource_request_headers
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
pass | ||
|
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 update_central_grafana_datasources
refactored with the following changes:
- Remove redundant pass statement (
remove-redundant-pass
)
@@ -17,6 +17,7 @@ | |||
Pull Request number, and the name of the *other* workflow that we want to provide | |||
a link to. | |||
""" | |||
|
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 56-92
refactored with the following changes:
- Remove unnecessary call to keys() [×2] (
remove-dict-keys
)
buf.append(escape_char) | ||
buf.append("%X" % byte) | ||
buf.extend((escape_char, "%X" % byte)) |
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 _escape_char
refactored with the following changes:
- Merge consecutive list appends into a single extend (
merge-list-appends-into-extend
)
completed_futures = 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 main
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Replace manual loop counter with call to enumerate (
convert-to-enumerate
)
c = yaml.load(test_cluster) | ||
return c | ||
return yaml.load(test_cluster) |
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 cluster
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
schema = DataFrameSchema( | ||
return DataFrameSchema( | ||
{ | ||
"project": Column(String), | ||
"total_with_credits": Column(Float, Check(lambda x: x > 0.0), coerce=True), | ||
"total_with_credits": Column( | ||
Float, Check(lambda x: x > 0.0), coerce=True | ||
), | ||
}, | ||
index=Index(DateTime, name="month"), | ||
) | ||
return schema |
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 schema
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
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!