-
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 #1
base: master
Are you sure you want to change the base?
Conversation
print("TreeCluster version %s" % VERSION); exit() | ||
print(f"TreeCluster version {VERSION}") | ||
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.
Lines 13-13
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
out = list(); i = 0; j = 0 | ||
out = [] | ||
i = 0 | ||
j = 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 merge_two_sorted_lists
refactored with the following changes:
- Replace
list()
with[]
(list-literal
)
out = list() | ||
out = [] |
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 merge_multi_sorted_lists
refactored with the following changes:
- Replace
list()
with[]
(list-literal
)
return x[int(len(x)/2)] | ||
return x[len(x) // 2] | ||
else: | ||
return (x[int(len(x)/2)]+x[int(len(x)/2)-1])/2 | ||
return (x[len(x) // 2] + x[len(x) // 2 - 1]) / 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 median
refactored with the following changes:
- Simplify division expressions [×3] (
simplify-division
)
cluster = list() | ||
descendants = Queue(); descendants.put(node) | ||
cluster = [] | ||
descendants = Queue() | ||
descendants.put(node) |
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 cut
refactored with the following changes:
- Replace
list()
with[]
(list-literal
)
clusters = list() | ||
clusters = [] |
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 length
refactored with the following changes:
- Replace
list()
with[]
(list-literal
)
q = Queue(); q.put(tree.root); roots = list() | ||
q = Queue() | ||
q.put(tree.root) | ||
roots = [] |
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 length_clade
refactored with the following changes:
- Replace
list()
with[]
(list-literal
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
clusters = list() | ||
clusters = [] |
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 root_dist
refactored with the following changes:
- Replace
list()
with[]
(list-literal
)
assert mode in modes, "Invalid mode. Must be one of: %s" % ', '.join(sorted(modes.keys())) | ||
assert ( | ||
mode in modes | ||
), f"Invalid mode. Must be one of: {', '.join(sorted(modes.keys()))}" |
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 leaf_dist
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
parser.add_argument('-m', '--method', required=False, type=str, default='max_clade', help="Clustering Method (options: %s)" % ', '.join(sorted(METHODS.keys()))) | ||
parser.add_argument('-tf', '--threshold_free', required=False, type=str, default=None, help="Threshold-Free Approach (options: %s)" % ', '.join(sorted(THRESHOLDFREE.keys()))) | ||
parser.add_argument( | ||
'-m', | ||
'--method', | ||
required=False, | ||
type=str, | ||
default='max_clade', | ||
help=f"Clustering Method (options: {', '.join(sorted(METHODS.keys()))})", | ||
) | ||
parser.add_argument( | ||
'-tf', | ||
'--threshold_free', | ||
required=False, | ||
type=str, | ||
default=None, | ||
help=f"Threshold-Free Approach (options: {', '.join(sorted(THRESHOLDFREE.keys()))})", | ||
) | ||
parser.add_argument('-v', '--verbose', action='store_true', help="Verbose Mode") | ||
parser.add_argument('--version', action='store_true', help="Display Version") | ||
args = parser.parse_args() | ||
assert args.method.lower() in METHODS, "ERROR: Invalid method: %s" % args.method | ||
assert args.threshold_free is None or args.threshold_free in THRESHOLDFREE, "ERROR: Invalid threshold-free approach: %s" % args.threshold_free | ||
assert args.method.lower() in METHODS, f"ERROR: Invalid method: {args.method}" | ||
assert ( | ||
args.threshold_free is None or args.threshold_free in THRESHOLDFREE | ||
), f"ERROR: Invalid threshold-free approach: {args.threshold_free}" |
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 579-612
refactored with the following changes:
- Replace interpolated string formatting with f-string [×4] (
replace-interpolation-with-fstring
) - Replace
list()
with[]
(list-literal
) - Replace if statement with if expression [×2] (
assign-if-exp
) - Remove unnecessary calls to
enumerate
when the index is not used (remove-unused-enumerate
)
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.
PR Type: Refactoring
Summary of PR: This PR introduces a series of refactoring changes aimed at improving code readability and performance. It includes the use of f-strings for string formatting, simplification of list initializations, and the removal of unnecessary semicolons. The changes also involve the use of more Pythonic constructs and adherence to best practices.
General PR suggestions
- Ensure that all external references such as
stderr
are properly imported or defined within the scope of the code to avoid runtime errors. - While the use of f-strings is a welcome improvement for readability and performance, be cautious with assertions that involve potentially large sets of keys. Consider pre-computing such sets outside of hot code paths to avoid performance penalties.
- Review the use of f-strings in assertions and error messages to ensure that they are clear and provide the necessary information for debugging.
- Double-check that the refactoring has not introduced any changes in the logic or behavior of the code, especially in complex expressions or control structures.
- Validate the refactoring against existing test cases to ensure that the behavior of the application remains consistent.
Your trial expires on December 18, 2023. Please email [email protected] to continue using Sourcery ✨
@@ -10,11 +10,14 @@ | |||
|
|||
# check if user is just printing version | |||
if '--version' in argv: | |||
print("TreeCluster version %s" % VERSION); exit() | |||
print(f"TreeCluster version {VERSION}") |
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.
suggestion (llm): Consider removing the semicolon at the end of the print statement for consistency with the rest of the codebase.
print(f"TreeCluster version {VERSION}") | |
print(f"TreeCluster version {VERSION}") |
@@ -210,7 +216,7 @@ def min_clusters_threshold_med_clade(tree,threshold,support): | |||
# if verbose, print the clades defined by each cluster | |||
if VERBOSE: | |||
for root in roots: | |||
print("%s;" % root.newick(), file=stderr) | |||
print(f"{root.newick()};", file=stderr) |
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.
suggestion (llm): Using f-strings is a great improvement for readability and performance. However, ensure that 'stderr' is imported from 'sys' or defined elsewhere in the code.
parser.add_argument('-v', '--verbose', action='store_true', help="Verbose Mode") | ||
parser.add_argument('--version', action='store_true', help="Display Version") | ||
args = parser.parse_args() | ||
assert args.method.lower() in METHODS, "ERROR: Invalid method: %s" % args.method | ||
assert args.threshold_free is None or args.threshold_free in THRESHOLDFREE, "ERROR: Invalid threshold-free approach: %s" % args.threshold_free | ||
assert args.method.lower() in METHODS, f"ERROR: Invalid method: {args.method}" |
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.
thought (llm): The use of f-strings for assertions is good for readability. However, consider the impact on performance if 'METHODS.keys()' is large, as it will be converted to a list and sorted every time this assertion is hit. If performance is a concern, pre-compute the sorted list of method keys outside the assertion.
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!