Sourcery refactored main branch#1
Conversation
| 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.
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.
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)
| def get_cutset(sources, targets, g): | ||
| source_sink_graph = g.copy() | ||
|
|
There was a problem hiding this comment.
Function get_cutset refactored with the following changes:
- Simplify logical expression using De Morgan identities (
de-morgan)
| style = style.lower() | ||
|
|
||
| if not (style in resources.BUILTIN_STYLES): | ||
| if style not in resources.BUILTIN_STYLES: |
There was a problem hiding this comment.
Function apply_builtin_style refactored with the following changes:
- Simplify logical expression using De Morgan identities [×2] (
de-morgan)
|
|
||
| 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.
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.
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.
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.
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.
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)
| module_path = os.path.abspath(__file__) | ||
| 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.
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.
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.pyfile, 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 ✨
|
|
||
| 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.
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] |
| _, 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.
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.
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'.
| 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.
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
mainrefactored 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
mainbranch, then run:Help us improve this pull request!