-
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 devel branch #2
base: devel
Are you sure you want to change the base?
Conversation
823a90b
to
27804d7
Compare
def get_args(cls, dist, header=None): # noqa: D205,D400 | ||
def get_args(cls, dist, header=None): # noqa: D205,D400 |
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_args
refactored with the following changes:
- Inline variable that is immediately yielded (
inline-immediately-yielded-variable
)
temp_val = val_obj.val if not isinstance(val_obj, (SerializableType, ArrayType)) else val_obj.formatted_val | ||
temp_val = ( | ||
val_obj.formatted_val | ||
if isinstance(val_obj, (SerializableType, ArrayType)) | ||
else val_obj.val | ||
) | ||
fmt_str = template.get_format_str() | ||
if temp_val is None: | ||
return "" | ||
if fmt_str: | ||
return format_string_template(fmt_str, (temp_val,)) | ||
return temp_val | ||
return format_string_template(fmt_str, (temp_val,)) if fmt_str else temp_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 ChData._compute_display_text
refactored with the following changes:
- Swap if/else branches of if expression to remove negation (
swap-if-expression
) - Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
if verbose and csv: | ||
return f"{time_str_nice},{raw_time_str},{ch_name},{self.id},{display_text}" | ||
if verbose and not csv: | ||
if verbose: | ||
if csv: | ||
return f"{time_str_nice},{raw_time_str},{ch_name},{self.id},{display_text}" | ||
return ( | ||
f"{time_str_nice}: {ch_name} ({self.id}) {raw_time_str} {display_text}" | ||
) | ||
if not verbose and csv: | ||
if csv: |
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 ChData.get_str
refactored with the following changes:
- Hoist a repeated condition into a parent condition (
hoist-repeated-if-condition
) - Remove redundant conditional [×2] (
remove-redundant-if
)
if verbose and csv: | ||
return f"{time_str},{raw_time_str},{name},{self.id},{arg_str}" | ||
if verbose and not csv: | ||
if verbose: | ||
if csv: | ||
return f"{time_str},{raw_time_str},{name},{self.id},{arg_str}" | ||
return f"{time_str}: {name} ({self.id}) {raw_time_str} : {arg_str}" | ||
if not verbose and csv: | ||
if csv: |
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 CmdData.get_str
refactored with the following changes:
- Hoist a repeated condition into a parent condition (
hoist-repeated-if-condition
) - Remove redundant conditional [×2] (
remove-redundant-if
)
self.display_text = event_temp.description if event_args is None else format_string_template( | ||
event_temp.format_str, tuple([arg.val for arg in event_args]) | ||
self.display_text = ( | ||
event_temp.description | ||
if event_args is None | ||
else format_string_template( | ||
event_temp.format_str, tuple(arg.val for arg in event_args) | ||
) |
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 EventData.__init__
refactored with the following changes:
- Replace unneeded comprehension with generator (
comprehension-to-generator
)
channel_list = test_api_utils.get_item_list( | ||
return test_api_utils.get_item_list( | ||
item_dictionary=project_dictionary.channel_id, | ||
search_filter=filter_predicate, | ||
template_to_data=ChData.get_empty_obj, | ||
) | ||
return channel_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 ChannelsCommand._get_item_list
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
closest_matches = difflib.get_close_matches(command_name, known_commands, n=num) | ||
return closest_matches | ||
return difflib.get_close_matches(command_name, known_commands, n=num) |
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 CommandSendCommand._get_closest_commands
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
cmd_description = item.get_description() | ||
if cmd_description: | ||
if cmd_description := item.get_description(): |
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 CommandSendCommand._get_item_string
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
close_matches = cls._get_closest_commands( | ||
if close_matches := cls._get_closest_commands( | ||
api.pipeline.dictionaries, command | ||
) | ||
if close_matches: | ||
): |
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 CommandSendCommand._execute_command
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
event_list = test_api_utils.get_item_list( | ||
return test_api_utils.get_item_list( | ||
item_dictionary=project_dictionary.event_id, | ||
search_filter=filter_predicate, | ||
template_to_data=EventData.get_empty_obj, | ||
) | ||
return event_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 EventsCommand._get_item_list
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
self.search_string = str(search_string) | ||
self.search_string = search_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 contains_search_string.__init__
refactored with the following changes:
- Remove unnecessary casts to int, str, float or bool (
remove-unnecessary-cast
)
new_args = func(*args) # lgtm [py/call/wrong-arguments] | ||
if new_args: | ||
if new_args := func(*args): |
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 repeat_until_interrupt
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
This removes the following comments ( why? ):
# lgtm [py/call/wrong-arguments]
if not ch == False or not ch == True: | ||
ch = True | ||
ch = 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 Channel.changed
refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if
) - Simplify logical expression using De Morgan identities [×2] (
de-morgan
)
vals = [0] + vals | ||
return vals | ||
return [0] + vals |
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 Event.deserialize
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
return [item for item in map(parseArg, args)] | ||
return list(map(parseArg, args)) |
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 SeqFileParser.parse
refactored with the following changes:
- Replace identity comprehension with call to collection constructor (
identity-comprehension
)
events_parser = parent_parser.add_parser( | ||
return parent_parser.add_parser( | ||
"events", | ||
description="print out new events that have occurred on the F Prime instance, sorted by timestamp", | ||
) | ||
return events_parser |
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 EventsSubparserInjector.create_subparser
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
flask_env = os.environ.copy() | ||
flask_env.update( | ||
{ | ||
"FLASK_APP": "fprime_gds.flask.app", | ||
"STANDARD_PIPELINE_ARGUMENTS": "|".join(reproduced_arguments), | ||
"SERVE_LOGS": "YES", | ||
} | ||
) | ||
flask_env = os.environ | { | ||
"FLASK_APP": "fprime_gds.flask.app", | ||
"STANDARD_PIPELINE_ARGUMENTS": "|".join(reproduced_arguments), | ||
"SERVE_LOGS": "YES", | ||
} |
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 launch_html
refactored with the following changes:
- Merge dictionary updates via the union operator [×2] (
dict-assign-update-to-union
) - Remove a redundant constructor in a dictionary union (
remove-redundant-constructor-in-dict-union
)
data = tlm_packet_size + packet[4 : 4 + size] | ||
|
||
return data | ||
return tlm_packet_size + packet[4 : 4 + size] |
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 ThreadedUDPRequestHandler.readData
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
except (KeyboardInterrupt, OSError, InterruptedError): | ||
except (KeyboardInterrupt, OSError): |
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 register_process_assassin
refactored with the following changes:
- Remove redundant exceptions from an except clause [×3] (
remove-redundant-exception
)
|
||
import sys | ||
|
||
PY3 = sys.version_info[0] == 3 | ||
|
||
if PY3: | ||
string_types = (str,) | ||
else: | ||
string_types = (basestring,) | ||
|
||
string_types = (str, ) if PY3 else (basestring, ) |
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 18-22
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
allow_extns = tuple(config.get(prefix + "ALLOW", ())) | ||
deny_extns = tuple(config.get(prefix + "DENY", ())) | ||
destination = config.get(prefix + "DEST") | ||
base_url = config.get(prefix + "URL") | ||
|
||
if destination is None: | ||
# the upload set's destination wasn't given | ||
if uset.default_dest: | ||
# use the "default_dest" callable | ||
destination = uset.default_dest(app) | ||
if destination is None: # still | ||
# use the default dest from the config | ||
if defaults["dest"] is not None: | ||
using_defaults = True | ||
destination = os.path.join(defaults["dest"], uset.name) | ||
else: | ||
raise RuntimeError(f"no destination for set {uset.name}") | ||
allow_extns = tuple(config.get(f"{prefix}ALLOW", ())) | ||
deny_extns = tuple(config.get(f"{prefix}DENY", ())) | ||
destination = config.get(f"{prefix}DEST") | ||
base_url = config.get(f"{prefix}URL") | ||
|
||
if destination is None and uset.default_dest: | ||
# use the "default_dest" callable | ||
destination = uset.default_dest(app) | ||
if destination is None: # still | ||
if defaults["dest"] is None: | ||
raise RuntimeError(f"no destination for set {uset.name}") | ||
|
||
using_defaults = True | ||
destination = os.path.join(defaults["dest"], uset.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 config_for_set
refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation
) - Merge nested if conditions (
merge-nested-ifs
) - Hoist conditional out of nested conditional (
hoist-if-from-if
) - Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
This removes the following comments ( why? ):
# use the default dest from the config
# the upload set's destination wasn't given
jsonable_dict.update({"name": input_type.__name__}) | ||
jsonable_dict["name"] = input_type.__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 jsonify_base_type
refactored with the following changes:
- Add single value to dictionary directly rather than using update() (
simplify-dictionary-update
)
event = EventData([], time_type.TimeType(), sample_template) | ||
return event | ||
return EventData([], time_type.TimeType(), sample_template) |
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 sample_event
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
event = EventData(tuple(), TimeType(), temp) | ||
return event | ||
return EventData(tuple(), TimeType(), temp) |
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 APITestCases.get_severity_event
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
assert True, "api raised the correct error" | ||
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 APITestCases.test_assert_telemetry
refactored with the following changes:
- Remove
assert True
statements [×2] (remove-assert-true
)
assert True, "api raised the correct error" | ||
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 APITestCases.test_assert_telemetry_sequence
refactored with the following changes:
- Remove
assert True
statements [×2] (remove-assert-true
)
assert True, "api raised the correct error" | ||
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 APITestCases.test_assert_telemetry_count
refactored with the following changes:
- Remove
assert True
statements [×2] (remove-assert-true
)
@@ -28,7 +28,6 @@ def check_str(pred): | |||
try: | |||
pred.__str__() | |||
print(pred) | |||
assert True, f"predicate provides string summary: {str(pred)}" |
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 PredicateTestCases.check_str
refactored with the following changes:
- Remove
assert True
statements (remove-assert-true
)
@@ -69,7 +68,6 @@ def test_Implemented(): | |||
str(pred) | |||
except NotImplementedError: | |||
assert False, "invoking __str__ on an complete subclass of predicate failed" | |||
assert True, "implemented predicate had no problems invoking functions" |
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 PredicateTestCases.test_Implemented
refactored with the following changes:
- Remove
assert True
statements (remove-assert-true
)
@@ -62,7 +62,7 @@ | |||
else: | |||
process.kill(signal.SIGINT) | |||
time.sleep(1) | |||
except (KeyboardInterrupt, OSError, InterruptedError): | |||
except (KeyboardInterrupt, OSError): |
Check notice
Code scanning / CodeQL
Empty except
@@ -71,13 +71,13 @@ | |||
process.kill() | |||
else: | |||
process.kill(signal.SIGKILL) | |||
except (KeyboardInterrupt, OSError, InterruptedError): | |||
except (KeyboardInterrupt, OSError): |
Check notice
Code scanning / CodeQL
Empty except
pass | ||
# Might as well close the log file because dead men tell no tales. | ||
try: | ||
if log is not None: | ||
log.close() | ||
except (KeyboardInterrupt, OSError, InterruptedError): | ||
except (KeyboardInterrupt, OSError): |
Check notice
Code scanning / CodeQL
Empty except
Branch
devel
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
devel
branch, then run:Help us improve this pull request!