Skip to content

feat: add synthesis pathway visualization utility#195

Draft
redrodeo03 wants to merge 4 commits into
mainfrom
visualise
Draft

feat: add synthesis pathway visualization utility#195
redrodeo03 wants to merge 4 commits into
mainfrom
visualise

Conversation

@redrodeo03
Copy link
Copy Markdown
Collaborator

Description

Fix #(issue)

image

This PR adds a function to generate images of pathways generated by DeepRetro
todo: add SVG

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
    • In this case, we recommend to discuss your modification on GitHub issues before creating the PR
  • Documentations (modification for documents)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New unit tests pass locally with my changes
  • I have checked my code and corrected any misspellings

@ARY2260 ARY2260 mentioned this pull request Apr 20, 2026
10 tasks
return root


def node_height(node: Node) -> int:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please docstrings, unit tests, fix linting and type annotations

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also add rst docs

Copy link
Copy Markdown
Collaborator

@ARY2260 ARY2260 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstrings and lint needed

return root


def node_height(node: Node) -> int:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also add rst docs

@riya-singh28 riya-singh28 marked this pull request as ready for review April 22, 2026 16:35
Copy link
Copy Markdown
Collaborator

@riya-singh28 riya-singh28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redrodeo03 I have left a few comments regarding doctests and suggested some changes in pytests.

Comment thread deepretro/tests/test_visualize.py Outdated
"product_metadata": {"chemical_formula": "C2H6O", "mass": 46.07},
})
assert formula == "C2H6O"
assert "46" in mass_str
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redrodeo03
assert "46" in mass_str is too loose a check. This may pass for incorrect values; consider asserting an exact value or using approximate float comparison.

assert "46" in mass_str


def test_mol_metadata_falls_back_to_rdkit() -> None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redrodeo03 Can you add a docstring explaining the test here?

assert img.mode == "RGB"


def test_visualize_pathway_smoke() -> None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redrodeo03, consider adding a test comparing the rendered image to a baseline image (preferably with tolerance to avoid brittle failures)

Comment thread deepretro/utils/visualize.py Outdated
@@ -0,0 +1,523 @@
"""Synthesis pathway visualization.

Renders retrosynthesis output from :class:`~deepretro.algorithms.autosolve.AutoSolver`
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please shift these doctrings to rst docs.

Node or None
Root node of the tree, or ``None`` if *result* has no steps.

Examples
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redrodeo03 Please run doctests locally; the current examples are missing required import statements

is_target=True,
)

def recurse(step_id: str) -> Node:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add docstrings


Examples
--------
>>> leaf = Node(label="Step 1", molecules=[{"smiles": "CCO"}])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing required import statements


Examples
--------
>>> root = Node(label="Step 0", molecules=[], is_target=True)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing import statements

Comment thread deepretro/utils/visualize.py Outdated

Examples
--------
>>> img = render_mol("CCO", 200) # doctest: +SKIP
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing required import statements

@redrodeo03 redrodeo03 marked this pull request as draft April 22, 2026 17:26
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.

3 participants