feat: draw/show SuperComponents in detail, expand it and show it's internal components in the visualisation diagram

[davidsbatista]

Related Issues

• fixes Add option to Pipeline.draw to allow expansion of a SuperComponent #9299

Proposed Changes:

• Added two new methods to the Pipeline
  • _find_super_components(self) - this will find all the SuperComponents in a Pipeline.
  • _merge_super_component_pipelines(self): this will expand all the SuperComponents in a pipeline. It will adjust the connections such that the internal components of a SuperComponent connect with the existing components of the Pipeline.

How did you test it?

• unit tests, integration tests, manual verification

Notes for the reviewer

In case you want to test it, start a local mermaid server

docker run --platform linux/amd64 --publish 3000:3000 --cap-add=SYS_ADMIN ghcr.io/jihchi/mermaid.ink

Generate an image for the original pipeline, and the version with the expanded components

from pathlib import Path

from haystack import Pipeline
from haystack.components.preprocessors import DocumentPreprocessor
from haystack.components.writers import DocumentWriter
from haystack.document_stores.in_memory import InMemoryDocumentStore

document_store = InMemoryDocumentStore()

pipeline = Pipeline()
pipeline.add_component("preprocessor", DocumentPreprocessor())
pipeline.add_component("writer", DocumentWriter(document_store = document_store))
pipeline.connect("preprocessor", "writer")

# original pipeline
out_file = Path("original_pipeline.png")
pipeline.draw(out_file, server_url="http://localhost:3000")

# merged pipeline that includes all components
out_file = Path("merged_pipeline.png")
pipeline.draw(out_file, server_url="http://localhost:3000",  super_component_expansion=True)

# component by component
pp = Pipeline()
pp.add_component("splitter", DocumentSplitter())
pp.add_component("cleaner", DocumentCleaner())
pp.add_component("writer", DocumentWriter(document_store = document_store))

pp.connect("splitter.documents", "cleaner.documents")
pp.connect("cleaner", "writer")

out_file = Path("component_by_component.png")
pipeline.draw(out_file, server_url="http://localhost:3000",  super_component_expansion=True)

SuperComponent

SuperComponent expanded (i.e.: super_component_expansion=True)

Pipeline with each components defined originaly

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added unit tests and updated the docstrings
• I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
• I documented my code
• I ran pre-commit hooks and fixed any issue

[coveralls]

Pull Request Test Coverage Report for Build 15196767034

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 67 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.1%) to 90.227%

---

Files with Coverage Reduction	New Missed Lines	%
core/pipeline/base.py	26	93.51%
core/pipeline/draw.py	41	74.53%

Totals
Change from base Build 15186721509:	-0.1%
Covered Lines:	11227
Relevant Lines:	12443

---

💛 - Coveralls

[sjrl]

Hey @davidsbatista could you also add the mermaid picture of your example? I think that would help with the review

[davidsbatista]

@sjrl I've also added a version of the same pipeline, but defining each component, no SuperComponents used

[sjrl]

Thanks @davidsbatista it's looking good! Looking back at the original issue do you think it would be possible to add this dotted box around where the super component is in the expanded view?

[davidsbatista]

@sjrl did you do that yourself or the community member? Just to understand if that was programmatically or "photoshopped"

[sjrl]

@sjrl did you do that yourself or the community member? Just to understand if that was programmatically or "photoshopped"

That was a community member. Originally from this comment.

[davidsbatista]

I explored a way to draw a line around it, but I still haven't found a nice solution. Would you suggest having the nodes coloured differently?

Example:

[sjrl]

Oh interesting, yeah I think any kind of visual difference would be nice! It would be great though if we could easily distinguish between different super components within the same pipeline. So maybe different shades of the same color? Also would it be possible to add a legend to indicate which color goes with which super component?

[vblagoje]

Looks nice - would be great to later extract utils related to super components in pipeline and component being a super component!

[sjrl]

We should add super_component_expansion at the bottom of this function otherwise inserting it in the middle is a breaking change.

[sjrl]

Same here we should move the new variable to the bottom of the function

[sjrl]

Nice the color coding and the legend look great! Just a few other minor comments.

[davidsbatista]

@sjrl I opted for forcing keyword arguments for draw() and show() - I think this is a better solution which also avoids problems if we need to add /remove any other parameters

[sjrl]

@sjrl I opted for forcing keyword arguments for draw() and show() - I think this is a better solution which also avoids problems if we need to add /remove any other parameters

I agree with you in principle, but enforcing keyword arguments now is also a breaking change right?

[davidsbatista]

@sjrl I opted for forcing keyword arguments for draw() and show() - I think this is a better solution which also avoids problems if we need to add /remove any other parameters

I agree with you in principle, but enforcing keyword arguments now is also a breaking change right?

I can try to write a wrapper so that we can ship this in the next release, with a warning. Then, after the 2.14 release remove the warning and the wrapper function.

[julian-risch]

Currently, we would show this warning even if there are no positional arguments. Can we show the warning only if there are any positional arguments? For example making use of inspect.getargvalues or with something similar to this?

[davidsbatista]

I would suggest to show it either way - just calling the function should be a enough to trigger the warning to the user that it will have some breaking changes in the future.

I don't think it's noisy if we show it even if he/she doesn't call it with positional arguments.

[sjrl]

I think I'm with Julian on this one. I think it makes more sense only to show the warning if the user is using the function in a way that won't work in the future.

Especially since if you follow the example how to fix the warning, the warning message still appears which could be frustrating to the user.

[davidsbatista]

I don't think it's good to shadow this from the user, especially if the user calls the function for whatever use he/she might have.

Anyway, I will code this only to trigger the warning in case the user calls the draw() or show() function using positional arguments.

There are more changes requested

[davidsbatista]

I've added a function decorator to check if the show() or draw() are ever called with positional arguments, and only in those cases a DeprecrationWarning is triggered - I've also added tests for all possible cases for the decorator function.

I added tests to calls to draw() and show() with positional arguments, without positional arguments to check if the warning is triggered and not triggered in the correct cases.

[julian-risch]

Looks good to me! 👍 Probably we will reuse the args_deprecated in future when we switch to kwargs only in other places too.