8000 Increasing the default X-axis spacing for flow plotting by mplachta · Pull Request #2967 · crewAIInc/crewAI · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Increasing the default X-axis spacing for flow plotting #2967

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 6, 2025
Merged

Increasing the default X-axis spacing for flow plotting #2967

merged 2 commits into from
Jun 6, 2025

Conversation

mplachta
Copy link
Contributor
@mplachta mplachta commented Jun 5, 2025

Before:
Screenshot 2025-06-05 at 3 07 21 PM

After:
Screenshot 2025-06-05 at 3 07 41 PM

@mplachta mplachta requested a review from lorenzejay June 5, 2025 22:08
@joaomdmoura
Copy link
Collaborator

Disclaimer: This review was made by a crew of AI Agents.

Code Review Comment for PR #2967 - visualization_utils.py

Overview

This PR introduces a significant change to the compute_positions function by doubling the default x-axis spacing parameter from 150 to 300 units. This adjustment aims to enhance the visual clarity of node arrangements in flow graph visualizations. The documentation has been updated accordingly, making the change well-communicated.

Code Improvements

  1. Configuration of Spacing Values: To enhance maintainability and flexibility, I recommend defining default values as constants. This will allow for easier updates in the future.

    DEFAULT_X_SPACING = 300
DEFAULT_Y_SPACING = 150

def compute_positions(
    flow: Any,
    node_levels: Dict[str, int],
    y_spacing: float = DEFAULT_Y_SPACING,
    x_spacing: float = DEFAULT_X_SPACING
) -> Dict[str, Tuple[float, float]]:

  2. Input Validation for Spacing Parameters: Implementing validation checks to ensure spacing parameters are positive can prevent runtime errors and enhance code robustness.

    if x_spacing <= 0 or y_spacing <= 0:
    raise ValueError("Spacing parameters must be positive numbers")

  3. Incorporate a Scale Factor Parameter: Adding a scale factor can facilitate adjustments for different visualization contexts without changing the core parameters.

    def compute_positions(
    flow: Any,
    node_levels: Dict[str, int],
    y_spacing: float = 150,
    x_spacing: float = 300,
    scale_factor: float = 1.0
) -> Dict[str, Tuple[float, float]]:

Historical Context

Previous PRs have adhered to a consistent spacing of 150 units, which balanced readability and aesthetics. Doubling the spacing to 300 units addresses potential overcrowding issues but may require additional evaluation when reviewing connected functions such as add_nodes_to_network and add_edges. It is crucial to maintain documentation for any changes in node spacing for future maintainers.

Possible Impact and Considerations

  1. Visual Readability: The increased spacing improves the visibility of nodes, especially in complex graphs where overcrowding might obscure connections.
  2. Performance Considerations: Larger flow graphs may impact rendering performance; thorough testing should be conducted post-implementation.
  3. Documentation and Regression Testing: It may be beneficial to document the rationale behind selecting 300 as the new spacing and include visual regression tests to ensure the modified parameters lead to desired improvements.

Conclusion

The modifications in this PR are straightforward and well-documented, effectively addressing readability issues in flow graph visualizations. While I find the changes acceptable, incorporating the recommendations provided above would enhance resilience and configurability.

I recommend approving this PR while encouraging the exploration of the suggested enhancements as future updates.

lorenzejay
lorenzejay approved these changes Jun 6, 2025
View reviewed changes
src/crewai/flow/visualization_utils.py
8000
@@ -140,7 +140,7 @@ def compute_positions(
flow: Any,
node_levels: Dict[str, int],
y_spacing: float = 150,
x_spacing: float = 150
x_spacing: float = 300
Copy link
Collaborator

Choose a reason for hiding this comment

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

love when you contribute @mplachta

@lorenzejay lorenzejay merged commit 02912a6 into crewAIInc:main Jun 6, 2025
7 checks passed
joaomdmoura pushed a commit that referenced this pull request Jun 8, 2025
@mplachta @joaomdmoura
Increasing the default X-axis spacing for flow plotting (#2967)
5537f3d 
* Increasing the default X-axis spacing for flow plotting

* removing unused imports
didier-durand pushed a commit to didier-durand/crewAI that referenced this pull request Jun 12, 2025
@mplachta @didier-durand
Increasing the default X-axis spacing for flow plotting (crewAIInc#2967)
49fd85d 
* Increasing the default X-axis spacing for flow plotting

* removing unused imports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorenzejay lorenzejay lorenzejay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mplachta @joaomdmoura @lorenzejay
0