Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

sourcery-ai[bot]
Copy link

@sourcery-ai sourcery-ai bot commented Dec 5, 2023

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:

git fetch origin sourcery/main
git merge --ff-only FETCH_HEAD
git reset HEAD^

Help us improve this pull request!

@sourcery-ai sourcery-ai bot requested a review from zagorGit December 5, 2023 11:18
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]
Copy link
Author

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:

Comment on lines -65 to +92
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
}
Copy link
Author

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:


Copy link
Author

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:
Copy link
Author

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]
Copy link
Author

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:

Comment on lines -278 to +272
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:
Copy link
Author

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:

Comment on lines -379 to +369
node_png_fname = create_png(node)
if node_png_fname:
if node_png_fname := create_png(node):
Copy link
Author

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:

if isinstance(x, str):
return x.split(',')
return None
return x.split(',') if isinstance(x, str) else None
Copy link
Author

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:

Comment on lines -19 to +41
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
}
Copy link
Author

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:

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)
Copy link
Author

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:

Copy link
Author

@sourcery-ai sourcery-ai bot left a 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]
Copy link
Author

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.

Suggested change
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(
Copy link
Author

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]
Copy link
Author

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:
Copy link
Author

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.

Suggested change
if source_sink_graph.has_node(node) and node not in sources:
if source_sink_graph.has_node(node):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants