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

feat: Enhance Pipeline.draw() to show image directly in Jupyter notebook #6961

Merged
merged 5 commits into from
Feb 9, 2024

Conversation

silvanocerza
Copy link
Contributor

Related Issues

Proposed Changes:

Change Pipeline.draw() to embed image if run in Jupyter notebook.
Now it can also be run without passing a path.
A path must still be provided if not running in a notebook, we raise a ValueError if missing.

How it appears on a Jupyter notebook ran in VS Code.
Screenshot 2024-02-08 at 15 34 15

How did you test it?

I added some new unit tests and adapted the existing ones. I also tested it manually on a local Jupyter notebook.

Notes for the reviewer

In this PR I also changed the Pipeline.draw() API to remove the choice of a rendering engine and always use mermaid.
Rendering with graphviz has been completely removed.

I also restructured a bit the draw package to avoid annoying repetitions in import paths.

Checklist

@silvanocerza silvanocerza added the 2.x Related to Haystack v2.0 label Feb 8, 2024
@silvanocerza silvanocerza self-assigned this Feb 8, 2024
@silvanocerza silvanocerza requested review from a team as code owners February 8, 2024 14:41
@silvanocerza silvanocerza requested review from dfokina and ZanSara and removed request for a team February 8, 2024 14:41
@coveralls
Copy link
Collaborator

coveralls commented Feb 8, 2024

Pull Request Test Coverage Report for Build 7843416947

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.4%) to 88.577%

Files with Coverage Reduction New Missed Lines %
core/pipeline/pipeline.py 21 92.66%
Totals Coverage Status
Change from base Build 7829729731: 0.4%
Covered Lines: 4769
Relevant Lines: 5384

💛 - Coveralls

@ZanSara ZanSara self-assigned this Feb 8, 2024
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

I have a doubt about the API, the rest looks good

haystack/core/pipeline/draw.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

✔️

@silvanocerza silvanocerza merged commit a7f36fd into main Feb 9, 2024
22 checks passed
@silvanocerza silvanocerza deleted the enhance-pipeline-draw branch February 9, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x Related to Haystack v2.0 topic:core topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When calling Pipeline.draw() in a notebook cell we should display the image directly
3 participants