-
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 main branch #1
base: main
Are you sure you want to change the base?
Conversation
to_remove_ranks = [] | ||
for r in ckn_ranks: | ||
if not (r in keep_edge_ranks): | ||
to_remove_ranks.append(r) | ||
|
||
to_remove_ranks = [r for r in ckn_ranks if r not in keep_edge_ranks] |
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 filter_ckn_edges
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
) - Simplify logical expression using De Morgan identities (
de-morgan
)
n for n, data in g.nodes(data=True) if not ( | ||
data['species'] in species | ||
) | ||
n | ||
for n, data in g.nodes(data=True) | ||
if data['species'] not in species | ||
] | ||
to_remove.update(no_species) | ||
reasons = {**reasons, **{n:"wrong species" for n in no_species if not n in reasons}} | ||
reasons = reasons | { | ||
n: "wrong species" for n in no_species if n not in reasons | ||
} | ||
|
||
if node_types: | ||
# nodes not in keep_types | ||
wrong_type = [n for n, data in g.nodes(data=True) if not (data['node_type'] in node_types)] | ||
wrong_type = [ | ||
n | ||
for n, data in g.nodes(data=True) | ||
if data['node_type'] not in node_types | ||
] | ||
to_remove.update(wrong_type) | ||
reasons = {**reasons, **{n:"wrong node type" for n in wrong_type if not n in reasons}} | ||
reasons = reasons | { | ||
n: "wrong node type" for n in wrong_type if n not in reasons | ||
} | ||
|
||
if tissues: | ||
wrong_tissue = [n for n, data in g.nodes(data=True) if ( not data['tissue'] ) or ( not (len([aa for aa in data['tissue'] if aa in tissues])>0) )] | ||
wrong_tissue = [ | ||
n | ||
for n, data in g.nodes(data=True) | ||
if not data['tissue'] | ||
or not [aa for aa in data['tissue'] if aa in tissues] | ||
] | ||
to_remove.update(wrong_tissue) | ||
reasons = {**reasons, **{n:"wrong tissue type" for n in wrong_tissue if not n in reasons}} | ||
reasons = reasons | { | ||
n: "wrong tissue type" for n in wrong_tissue if n not in reasons | ||
} |
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 filter_ckn_nodes
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×7] (
de-morgan
) - Applies the dictionary union operator where applicable [×5] (
use-dictionary-union
) - Simplify sequence length comparison (
simplify-len-comparison
)
|
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_cutset
refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan
)
if not (style in resources.BUILTIN_STYLES): | ||
if style not in resources.BUILTIN_STYLES: |
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 apply_builtin_style
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan
)
node_names = [n for n in node_names if not n in skip_nodes] | ||
node_names = [n for n in node_names if n not in skip_nodes] |
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 highlight_path
refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension
) - Simplify logical expression using De Morgan identities (
de-morgan
) - Replace identity comprehension with call to collection constructor (
identity-comprehension
)
if not (node in skip_source): | ||
if node not in skip_source: | ||
path_searches_attributes[node]['node-path-source'] = source | ||
skip_source.update([node]) | ||
|
||
_, cytoscape_edges = get_path_edges(paths, g) | ||
for e in cytoscape_edges: | ||
if not e in edge_priority: | ||
if e not in edge_priority: |
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 apply_shortest_paths_style
refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan
) - Replace unneeded comprehension with generator (
comprehension-to-generator
)
node_png_fname = create_png(node) | ||
if node_png_fname: | ||
if node_png_fname := create_png(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 add_custom_png
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
if isinstance(x, str): | ||
return x.split(',') | ||
return None | ||
return x.split(',') if isinstance(x, str) else None |
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 pss_to_networkx
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
)
n for n, data in g.nodes(data=True) if not ( | ||
len([h for h in homologue_properties if data[h]])>0 | ||
or | ||
not (data['node_type'] in ['PlantCoding', 'PlantNonCoding']) | ||
n | ||
for n, data in g.nodes(data=True) | ||
if not ( | ||
[h for h in homologue_properties if data[h]] | ||
or data['node_type'] not in ['PlantCoding', 'PlantNonCoding'] | ||
) | ||
] | ||
to_remove.update(no_species) | ||
reasons = {**reasons, **{n:"species missing" for n in no_species if not n in reasons}} | ||
reasons = reasons | { | ||
n: "species missing" for n in no_species if n not in reasons | ||
} | ||
|
||
if node_types: | ||
# nodes not in keep_types | ||
wrong_type = [n for n, data in g.nodes(data=True) if not (data['node_type'] in node_types)] | ||
wrong_type = [ | ||
n | ||
for n, data in g.nodes(data=True) | ||
if data['node_type'] not in node_types | ||
] | ||
to_remove.update(wrong_type) | ||
reasons = {**reasons, **{n:"wrong node type" for n in wrong_type if not n in reasons}} | ||
reasons = reasons | { | ||
n: "wrong node type" for n in wrong_type if n not in reasons | ||
} |
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 filter_pss_nodes
refactored with the following changes:
- Simplify sequence length comparison (
simplify-len-comparison
) - Simplify logical expression using De Morgan identities [×6] (
de-morgan
) - Applies the dictionary union operator where applicable [×4] (
use-dictionary-union
)
resource_path = os.path.join(os.path.dirname(module_path), STYLE_XML) | ||
return resource_path | ||
return os.path.join(os.path.dirname(module_path), STYLE_XML) |
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_style_xml_path
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
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 simplifying and improving the readability of the code. It includes the use of list comprehensions, set operations, and direct returns from functions to streamline the codebase.
General PR suggestions
- Review the changes to ensure that the refactoring does not introduce any logical changes that could affect the functionality, especially where direct returns are used in place of variable assignments.
- Consider the performance implications of the changes, such as the suggestion to use a set for membership checks in lists that could potentially be large.
- Verify that the removal of redundant conditions, as pointed out in the
cuts.py
file, does not omit any necessary logic that might have been intended but not clearly expressed in the original code. - Ensure that the refactoring aligns with the overall coding style and standards of the project, maintaining consistency across the codebase.
- Since the PR is generated by an automated tool, it might be beneficial to manually review the changes to ensure that they make sense in the context of the entire application and do not introduce any unintended side effects.
Your trial expires on December 19, 2023. Please email tim@sourcery.ai to continue using Sourcery ✨
@@ -90,7 +90,7 @@ def highlight_path(node_names, colour, skip_nodes=None, skip_edges=None, label_c | |||
''' | |||
|
|||
if isinstance(skip_nodes, Sequence) and not isinstance(skip_nodes, str): | |||
node_names = [n for n in node_names if not n in skip_nodes] | |||
node_names = [n for n in node_names if n not in skip_nodes] |
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 using a set for 'skip_nodes' if it's expected to be large for performance benefits when checking membership.
node_names = [n for n in node_names if n not in skip_nodes] | |
skip_nodes_set = set(skip_nodes) | |
node_names = [n for n in node_names if n not in skip_nodes_set] |
@@ -180,17 +178,15 @@ def subnetwork_edge_induced_from_paths(paths, g, parent_suid, name="subnetwork ( | |||
|
|||
_, cytoscape_edges = get_path_edges(paths, g) | |||
|
|||
network_suid = p4c.networks.create_subnetwork( | |||
return p4c.networks.create_subnetwork( |
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): Directly returning the result of 'create_subnetwork' is concise, but ensure that no additional processing on 'network_suid' is required before returning.
if not (r in keep_edge_ranks): | ||
to_remove_ranks.append(r) | ||
|
||
to_remove_ranks = [r for r in ckn_ranks if r not in keep_edge_ranks] |
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 refactoring to a list comprehension is cleaner, but consider if 'keep_edge_ranks' would benefit from being a set for performance reasons similar to the previous comment on 'skip_nodes'.
@@ -15,23 +15,23 @@ def get_cutset(sources, targets, g): | |||
|
|||
source_sink_graph.add_node('sink') | |||
for node in targets: | |||
if source_sink_graph.has_node(node) and not (node in sources): | |||
if source_sink_graph.has_node(node) and node not in sources: |
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.
issue (llm): The check 'node not in sources' is redundant because it's already implied by the surrounding 'for' loop iterating over 'targets'. This could be removed to simplify the condition.
if source_sink_graph.has_node(node) and node not in sources: | |
if source_sink_graph.has_node(node): |
Branch
main
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
main
branch, then run:Help us improve this pull request!