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

Straight line edge conversion for (poly)lines and improved curve conversion #81

Merged
merged 9 commits into from
Jul 6, 2024

Conversation

hbmartin
Copy link
Owner

@hbmartin hbmartin commented Jul 5, 2024

Summary by Sourcery

This pull request introduces significant improvements to curve handling in the graphviz2drawio project. It adds support for straight line and cubic Bézier curve conversions, enhances the Curve and MxGraph classes for better flexibility and robustness, and includes new utility functions for cubic Bézier curve operations. Additionally, new tests have been added to ensure the correctness of these features, and unused code has been cleaned up.

  • New Features:
    • Introduced support for straight line and cubic Bézier curve conversion in the Curve class.
    • Added methods for calculating cubic Bézier curve derivatives and splitting cubic Bézier curves into quadratics.
    • Implemented new utility functions for handling cubic Bézier curves, including inflection point subdivision and tangent intersection approximation.
  • Enhancements:
    • Refactored Curve class to use a more flexible representation of curves, supporting both straight lines and cubic Bézier curves.
    • Improved edge handling in MxGraph to better support different curve types and styles.
    • Enhanced CurveFactory to handle SVG path parsing more robustly, including support for quadratic Bézier curves.
  • Tests:
    • Added new tests for line calculation and vertical line handling in the Curve class.
    • Introduced a new test for curved polylines to ensure proper conversion and rendering.
  • Chores:
    • Removed unused LinearRegression module from the project.

Summary by CodeRabbit

  • New Features

    • Added support for cubic Bézier curves for more flexible and accurate graph representation.
  • Bug Fixes

    • Ensured undirected graphs are not drawn with arrows.
  • Documentation

    • Updated README to include a task for ensuring undirected graphs are not drawn with arrows.
  • Tests

    • Added new tests for Bézier curves and curved polylines to ensure robustness and accuracy.
  • Refactor

    • Improved type hints in methods for better code clarity and maintainability.
    • Simplified logic handling for curves and edge styles in various modules.
  • Chores

    • Added .vscode to .gitignore to avoid unintentional commits of IDE-specific files.

Copy link
Contributor

sourcery-ai bot commented Jul 5, 2024

Reviewer's Guide by Sourcery

This pull request refactors the curve handling in the graphviz2drawio project to use complex numbers for start, end, and control points. It introduces new Bézier curve operations, updates edge handling and styling, and adds new test cases. The LinearRegression module has been removed in favor of a math-based linearity check.

File-Level Changes

Files Changes
graphviz2drawio/mx/Curve.py
graphviz2drawio/mx/CurveFactory.py
graphviz2drawio/mx/bezier.py
Refactored curve handling to use complex numbers and added new Bézier curve operations.
graphviz2drawio/mx/MxGraph.py
graphviz2drawio/mx/Edge.py
graphviz2drawio/mx/MxConst.py
graphviz2drawio/mx/Styles.py
Updated edge handling and styling to support new curve structure and improve edge ID generation.
test/test_graphs.py
test/test_curve.py
Added new test cases for curved polylines and line function.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

coderabbitai bot commented Jul 5, 2024

Warning

Rate limit exceeded

@hbmartin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 0 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 48b3a70 and 9cec411.

Walkthrough

The changes encompass enhancements to the graphviz2drawio utility, including updates to Graphviz to Draw.io graph conversion, emphasizing undirected graph visualizations without arrows. Significant updates include import refactoring, type hints, new constants, additional functionalities for Bézier curves, and comprehensive unit tests. The updates aim to refine edge representation, improve method efficiencies, and streamline style configurations.

Changes

File Change Summary
.gitignore Added .vscode to the ignored items list.
README.md Added a checklist item to ensure undirected graphs are rendered without arrows.
graphviz2drawio/graphviz2drawio.py Refactored import for parse_nodes_edges_clusters to import from models.SvgParser.
graphviz2drawio/models/CoordsTranslate.py Added type hints for complex_translate method.
graphviz2drawio/models/__init__.py Removed imports for SvgParser and parse_nodes_edges_clusters.
graphviz2drawio/mx/Curve.py Overhauled Curve class to include handling of cubic Bézier curves and related functionalities.
graphviz2drawio/mx/CurveFactory.py Updated methods and logic in CurveFactory, including new Bézier curve functionalities.
graphviz2drawio/mx/Edge.py Renamed style attribute to line_style and updated accordingly.
graphviz2drawio/mx/MxConst.py Added CURVED and SHARP constants.
graphviz2drawio/mx/MxGraph.py Revamped edge addition and style handling methods, improving curve support and id generation.
graphviz2drawio/mx/Styles.py Modified EDGE style declaration, removing certain attributes and focusing on essential ones.
graphviz2drawio/mx/bezier.py Introduced functions to handle cubic Bézier curves, including control points and approximation.
test/test_curve.py Added tests for the _line function handling line computations.
test/test_graphs.py Enhanced test_polylines() and added test_polylines_curved() for curved polylines.
test/undirected/polylines_curved.gv.txt Added graph defining nodes and edges configurations with ranks.

Poem

In the world of graphs anew,
Curves and lines align, it's true,
From Bézier's dance to edges proud,
Styles refined, we stand endowed.

By rabbit's code, perfection sought,
A graph's pure form, now finely wrought. 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@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.

Hey @hbmartin - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Incorrect use of p.real and p.imag (link)
  • Remove or replace the placeholder assertion (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
  • Potential hard-coded secret found. (link)
Here's what I looked at during the review
  • 🔴 General issues: 1 blocking issue, 7 other issues
  • 🔴 Security: 9 blocking issues
  • 🔴 Testing: 1 blocking issue, 3 other issues
  • 🟢 Complexity: all looks good
  • 🟡 Documentation: 1 issue found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/MxGraph.py Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/CurveFactory.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 19

Outside diff range and nitpick comments (15)
graphviz2drawio/mx/CurveFactory.py (5)

1-2: Remove unnecessary blank line.

The blank line between the import statements is not necessary and can be removed for consistency.

- import cmath
-
+ import cmath

7-7: Prefer absolute imports over relative imports.

Replace relative imports from parent modules with absolute imports for better readability and maintainability.

- from ..models.CoordsTranslate import CoordsTranslate
+ from graphviz2drawio.models.CoordsTranslate import CoordsTranslate
Tools
Ruff

7-7: Prefer absolute imports over relative imports from parent modules

Replace relative imports from parent modules with absolute imports

(TID252)


28-28: Add trailing comma for consistency.

Add a trailing comma after the last element in the list to maintain consistency.

- p.start, p.control1, p.control2, p.end
+ p.start, p.control1, p.control2, p.end,
Tools
Ruff

28-28: Trailing comma missing

Add trailing comma

(COM812)


31-35: Add trailing commas for consistency.

Add trailing commas after the last element in the list to maintain consistency.

- approximate_cubic_bezier_as_quadratic(*cube)[1]
+ approximate_cubic_bezier_as_quadratic(*cube)[1],
Tools
Ruff

31-35: Use a list comprehension to create a transformed list

(PERF401)


33-33: Trailing comma missing

Add trailing comma

(COM812)


34-34: Trailing comma missing

Add trailing comma

(COM812)


46-46: Add trailing newline.

Add a trailing newline at the end of the file for better readability and to follow best practices.

- return isinstance(p, CubicBezier)
+ return isinstance(p, CubicBezier)
+
Tools
Ruff

46-46: No newline at end of file

Add trailing newline

(W292)

README.md (1)

90-93: Use asterisk for unordered list style.

The unordered list style should be consistent. Replace the dash with an asterisk.

- - [ ] Ensure undirected graphs are not drawn with arrows
- - [ ] Run ruff in CI
- - [ ] Publish api docs to GH pages
- - [ ] Restore codecov to test GHA
+ * [ ] Ensure undirected graphs are not drawn with arrows
+ * [ ] Run ruff in CI
+ * [ ] Publish api docs to GH pages
+ * [ ] Restore codecov to test GHA
Tools
Markdownlint

90-90: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


91-91: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


92-92: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


93-93: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

test/test_graphs.py (1)

104-104: Uncomment or add a TODO comment for the assertion.

The commented-out assertion should either be uncommented or a TODO comment should be added to indicate the need for implementation.

- # assert False
+ # TODO: Implement assertion for test_polylines_curved
graphviz2drawio/mx/MxGraph.py (3)

5-6: Remove unnecessary blank line.

The blank line between the import statements is not necessary and can be removed for consistency.

- from graphviz2drawio.mx.Edge import Edge
-
+ from graphviz2drawio.mx.Edge import Edge

46-46: Add a docstring for the method.

The method lacks a docstring. Adding a docstring will improve readability and maintainability.

- def add_edge(self, edge: Edge) -> None:
+ def add_edge(self, edge: Edge) -> None:
+     """
+     Add an edge to the graph.
+     """

117-117: Add trailing comma for consistency.

Add a trailing comma after the last element in the list to maintain consistency.

- element, MxConst.GEO, attrib={"as": "geometry", "relative": "1"}
+ element, MxConst.GEO, attrib={"as": "geometry", "relative": "1"},
Tools
Ruff

117-117: Trailing comma missing

Add trailing comma

(COM812)

graphviz2drawio/mx/Curve.py (2)

17-17: Add trailing comma.

Add a trailing comma for consistency and to avoid syntax errors in future changes.

- self, start: complex, end: complex, is_bezier: bool, points: list[complex]
+ self, start: complex, end: complex, is_bezier: bool, points: list[complex],

128-128: Add trailing comma.

Add a trailing comma for consistency and to avoid syntax errors in future changes.

- def _lower_cubic_bezier_to_two_quadratics(P0: complex, P1: complex, P2: complex, P3: complex, t: float) -> tuple[list[complex], list[complex]]:
+ def _lower_cubic_bezier_to_two_quadratics(p0: complex, p1: complex, p2: complex, p3: complex, t: float,) -> tuple[list[complex], list[complex]]:
Tools
Ruff

128-128: Argument name P0 should be lowercase

(N803)


128-128: Argument name P1 should be lowercase

(N803)


128-128: Argument name P2 should be lowercase

(N803)


128-128: Argument name P3 should be lowercase

(N803)


128-128: Trailing comma missing

Add trailing comma

(COM812)

graphviz2drawio/mx/bezier.py (3)

9-9: Add trailing comma.

Add a trailing comma for consistency and to avoid syntax errors in future changes.

- p1: complex, c1: complex, c2: complex, p2: complex, t: float
+ p1: complex, c1: complex, c2: complex, p2: complex, t: float,
Tools
Ruff

9-9: Trailing comma missing

Add trailing comma

(COM812)


35-35: Add trailing comma.

Add a trailing comma for consistency and to avoid syntax errors in future changes.

- p1: complex, c1: complex, c2: complex, p2: complex, t: float
+ p1: complex, c1: complex, c2: complex, p2: complex, t: float,
Tools
Ruff

35-35: Trailing comma missing

Add trailing comma

(COM812)


167-167: Add trailing comma.

Add a trailing comma for consistency and to avoid syntax errors in future changes.

- p0: complex, c1: complex, c2: complex, p2: complex
+ p0: complex, c1: complex, c2: complex, p2: complex,
Tools
Ruff

167-167: Trailing comma missing

Add trailing comma

(COM812)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2531762 and b922bf0.

Files selected for processing (15)
  • .gitignore (1 hunks)
  • README.md (1 hunks)
  • graphviz2drawio/graphviz2drawio.py (1 hunks)
  • graphviz2drawio/models/CoordsTranslate.py (1 hunks)
  • graphviz2drawio/models/init.py (1 hunks)
  • graphviz2drawio/mx/Curve.py (3 hunks)
  • graphviz2drawio/mx/CurveFactory.py (1 hunks)
  • graphviz2drawio/mx/Edge.py (2 hunks)
  • graphviz2drawio/mx/MxConst.py (1 hunks)
  • graphviz2drawio/mx/MxGraph.py (7 hunks)
  • graphviz2drawio/mx/Styles.py (1 hunks)
  • graphviz2drawio/mx/bezier.py (1 hunks)
  • test/test_curve.py (1 hunks)
  • test/test_graphs.py (1 hunks)
  • test/undirected/polylines_curved.gv.txt (1 hunks)
Files skipped from review due to trivial changes (5)
  • .gitignore
  • graphviz2drawio/models/CoordsTranslate.py
  • graphviz2drawio/mx/MxConst.py
  • graphviz2drawio/mx/Styles.py
  • test/undirected/polylines_curved.gv.txt
Additional context used
Ruff
graphviz2drawio/mx/CurveFactory.py

7-7: Prefer absolute imports over relative imports from parent modules

Replace relative imports from parent modules with absolute imports

(TID252)


28-28: Trailing comma missing

Add trailing comma

(COM812)


31-35: Use a list comprehension to create a transformed list

(PERF401)


33-33: Trailing comma missing

Add trailing comma

(COM812)


34-34: Trailing comma missing

Add trailing comma

(COM812)


46-46: No newline at end of file

Add trailing newline

(W292)

test/test_graphs.py

95-95: Assertion always fails, replace with pytest.fail()

(PT015)


95-95: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

graphviz2drawio/mx/MxGraph.py

64-64: Unused static method argument: source_node

(ARG004)


64-64: Unused static method argument: target_node

(ARG004)


117-117: Trailing comma missing

Add trailing comma

(COM812)

graphviz2drawio/mx/Curve.py

2-2: Import from collections.abc instead: Callable

Import from collections.abc

(UP035)


2-2: typing.Any imported but unused

Remove unused import: typing.Any

(F401)


16-16: Boolean-typed positional argument in function definition

(FBT001)


16-16: Trailing comma missing

Add trailing comma

(COM812)


25-25: Found commented-out code

Remove commented-out code

(ERA001)


128-128: Argument name P0 should be lowercase

(N803)


128-128: Argument name P1 should be lowercase

(N803)


128-128: Argument name P2 should be lowercase

(N803)


128-128: Argument name P3 should be lowercase

(N803)


128-128: Trailing comma missing

Add trailing comma

(COM812)


130-131: 1 blank line required between summary line and description

(D205)


130-131: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)


130-131: First line of docstring should be in imperative mood: "Splits a cubic Bézier curve defined by control points P0-P3 at t,"

(D401)


132-132: Variable Q0 in function should be lowercase

(N806)


133-133: Variable Q1 in function should be lowercase

(N806)


134-134: Variable Q2 in function should be lowercase

(N806)


138-138: Variable Q3 in function should be lowercase

(N806)


151-164: Found useless expression. Either assign it to a variable or remove it.

(B018)

graphviz2drawio/mx/bezier.py

9-9: Trailing comma missing

Add trailing comma

(COM812)


15-15: Missing blank line after last section ("Returns")

Add blank line after "Returns"

(D413)


15-15: Missing dashed underline after section ("Returns")

Add dashed line under "Returns"

(D407)


15-15: Section name should end with a newline ("Returns")

Add newline after "Returns"

(D406)


35-35: Trailing comma missing

Add trailing comma

(COM812)


42-42: Missing blank line after last section ("Returns")

Add blank line after "Returns"

(D413)


42-42: Missing dashed underline after section ("Returns")

Add dashed line under "Returns"

(D407)


42-42: Section name should end with a newline ("Returns")

Add newline after "Returns"

(D406)


46-46: Exception must not use an f-string literal, assign to variable first

Assign to variable; remove f-string literal

(EM102)


56-56: Missing blank line after last section ("Returns")

Add blank line after "Returns"

(D413)


56-56: Missing dashed underline after section ("Returns")

Add dashed line under "Returns"

(D407)


56-56: Section name should end with a newline ("Returns")

Add newline after "Returns"

(D406)


100-100: Missing blank line after last section ("Returns")

Add blank line after "Returns"

(D413)


100-100: Missing dashed underline after section ("Returns")

Add dashed line under "Returns"

(D407)


100-100: Section name should end with a newline ("Returns")

Add newline after "Returns"

(D406)


134-134: Missing issue link on the line following this TODO

(TD003)


134-134: Line contains TODO, consider resolving the issue

(FIX002)


150-150: Missing issue link on the line following this TODO

(TD003)


150-150: Line contains TODO, consider resolving the issue

(FIX002)


167-167: Trailing comma missing

Add trailing comma

(COM812)


169-170: No blank lines allowed after function docstring (found 1)

Remove blank line(s) after function docstring

(D202)


169-170: 1 blank line required between summary line and description

(D205)


169-170: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)

Markdownlint
README.md

87-87: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


88-88: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


89-89: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


90-90: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


91-91: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


92-92: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


93-93: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

Additional comments not posted (9)
graphviz2drawio/models/__init__.py (1)

Line range hint 4-4: Changes approved.

The removal of the import statement for SvgParser is appropriate and aligns with the updated import in graphviz2drawio.py.

test/test_curve.py (3)

1-1: Import statement looks good.

The import statement for _line from graphviz2drawio.mx.Curve is correct.


4-9: Test for _line function is well-implemented.

The test covers the horizontal line case and verifies the function's output at different points.


11-13: Test for vertical line case is appropriate.

The test correctly handles the case where the line is vertical, ensuring the function returns None.

graphviz2drawio/graphviz2drawio.py (2)

5-5: Import statement updated correctly.

The import statement has been updated to import parse_nodes_edges_clusters from models.SvgParser, which is appropriate given the removal from __init__.py.


Line range hint 8-29: Changes in convert function look good.

The function correctly uses the updated import and processes the graph as expected.

graphviz2drawio/mx/Edge.py (2)

20-20: Attribute renaming is appropriate.

The style attribute has been renamed to line_style, which improves clarity.


37-37: Reference update in __repr__ method is correct.

The __repr__ method correctly references the renamed line_style attribute.

graphviz2drawio/mx/bezier.py (1)

169-170: Remove blank line after docstring.

Remove the blank line after the function docstring to adhere to PEP 257.

- """Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2)
- 
+ """
+ Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2)

Likely invalid or redundant comment.

Tools
Ruff

169-170: No blank lines allowed after function docstring (found 1)

Remove blank line(s) after function docstring

(D202)


169-170: 1 blank line required between summary line and description

(D205)


169-170: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)

graphviz2drawio/mx/CurveFactory.py Outdated Show resolved Hide resolved
test/test_graphs.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/MxGraph.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
Comment on lines 169 to 170
"""Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2)
using tangent intersection."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix docstring formatting.

Ensure proper formatting of docstrings for better readability and adherence to PEP 257.

- """Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2)
- using tangent intersection.
- """
+ """
+ Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2) using tangent intersection.
+ """
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2)
using tangent intersection."""
"""
Approximates a cubic Bézier curve (p0-p2) with a quadratic Bézier curve (Q0-Q2) using tangent intersection.
"""
Tools
Ruff

169-170: No blank lines allowed after function docstring (found 1)

Remove blank line(s) after function docstring

(D202)


169-170: 1 blank line required between summary line and description

(D205)


169-170: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)

graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/bezier.py Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b922bf0 and a000bb1.

Files selected for processing (1)
  • graphviz2drawio/mx/Curve.py (3 hunks)
Additional context used
Ruff
graphviz2drawio/mx/Curve.py

2-2: Import from collections.abc instead: Callable

Import from collections.abc

(UP035)


2-2: typing.Any imported but unused

Remove unused import: typing.Any

(F401)


16-16: Trailing comma missing

Add trailing comma

(COM812)


25-25: Found commented-out code

Remove commented-out code

(ERA001)


128-128: Argument name P0 should be lowercase

(N803)


128-128: Argument name P1 should be lowercase

(N803)


128-128: Argument name P2 should be lowercase

(N803)


128-128: Argument name P3 should be lowercase

(N803)


128-128: Trailing comma missing

Add trailing comma

(COM812)


130-131: 1 blank line required between summary line and description

(D205)


130-131: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)


130-131: First line of docstring should be in imperative mood: "Splits a cubic Bézier curve defined by control points P0-P3 at t,"

(D401)


132-132: Variable Q0 in function should be lowercase

(N806)


133-133: Variable Q1 in function should be lowercase

(N806)


134-134: Variable Q2 in function should be lowercase

(N806)


138-138: Variable Q3 in function should be lowercase

(N806)


151-164: Found useless expression. Either assign it to a variable or remove it.

(B018)

Additional comments not posted (11)
graphviz2drawio/mx/Curve.py (11)

8-13: LGTM!

The class docstring is clear and informative.


29-46: Potential issue with _line function

The _line function might return None if the start and end points have the same real part. Ensure this case is handled correctly.


39-41: Consider making rel_tol a parameter

The relative tolerance value rel_tol is hardcoded. Consider making it a parameter to allow for more flexibility.


57-64: LGTM!

The cubic_bezier_derivative method calculates the derivative correctly.


80-81: Incorrect use of p.real and p.imag

The _cubic_bezier function should use the list indices for control points instead of p.real and p.imag.


87-100: LGTM!

The _derivative_of_cubic_bezier method calculates the derivative correctly.


33-35: Potential issue with _line function

The _line function might return None if the start and end points have the same real part. Ensure this case is handled correctly.


128-128: Use lowercase argument names.

Argument names should be lowercase to follow PEP 8 naming conventions.

- def _lower_cubic_bezier_to_two_quadratics(P0: complex, P1: complex, P2: complex, P3: complex, t: float) -> tuple[list[complex], list[complex]]:
+ def _lower_cubic_bezier_to_two_quadratics(p0: complex, p1: complex, p2: complex, p3: complex, t: float) -> tuple[list[complex], list[complex]]:
Tools
Ruff

128-128: Argument name P0 should be lowercase

(N803)


128-128: Argument name P1 should be lowercase

(N803)


128-128: Argument name P2 should be lowercase

(N803)


128-128: Argument name P3 should be lowercase

(N803)


128-128: Trailing comma missing

Add trailing comma

(COM812)


130-131: Fix docstring formatting.

Ensure proper formatting of docstrings for better readability and adherence to PEP 257.

- """Splits a cubic Bézier curve defined by control points P0-P3 at t,
- returning two new cubic Bézier curve segments."""
+ """
+ Split a cubic Bézier curve defined by control points p0-p3 at t, returning two new cubic Bézier curve segments.
+ 
+ """
Tools
Ruff

130-131: 1 blank line required between summary line and description

(D205)


130-131: Multi-line docstring closing quotes should be on a separate line

Move closing quotes to new line

(D209)


130-131: First line of docstring should be in imperative mood: "Splits a cubic Bézier curve defined by control points P0-P3 at t,"

(D401)


132-147: Use lowercase variable names.

Variable names should be lowercase to follow PEP 8 naming conventions.

- Q0 = P0
- Q1 = complex((1 - t) * P0.real + t * P1.real, (1 - t) * P0.imag + t * P1.imag)
- Q2 = complex((1 - t) ** 2 * P0.real + 2 * (1 - t) * t * P1.real + t**2 * P2.real, (1 - t) ** 2 * P0.imag + 2 * (1 - t) * t * P1.imag + t**2 * P2.imag,)
- Q3 = complex((1 - t) ** 3 * P0.real + 3 * (1 - t) ** 2 * t * P1.real + 3 * (1 - t) * t**2 * P2.real + t**3 * P3.real, (1 - t) ** 3 * P0.imag + 3 * (1 - t) ** 2 * t * P1.imag + 3 * (1 - t) * t**2 * P2.imag + t**3 * P3.imag,)
- return [Q0, Q1, Q2], [Q2, Q3, P3]
+ q0 = p0
+ q1 = complex((1 - t) * p0.real + t * p1.real, (1 - t) * p0.imag + t * p1.imag)
+ q2 = complex((1 - t) ** 2 * p0.real + 2 * (1 - t) * t * p1.real + t**2 * p2.real, (1 - t) ** 2 * p0.imag + 2 * (1 - t) * t * p1.imag + t**2 * p2.imag,)
+ q3 = complex((1 - t) ** 3 * p0.real + 3 * (1 - t) ** 2 * t * p1.real + 3 * (1 - t) * t**2 * p2.real + t**3 * p3.real, (1 - t) ** 3 * p0.imag + 3 * (1 - t) ** 2 * t * p1.imag + 3 * (1 - t) * t**2 * p2.imag + t**3 * p3.imag,)
+ return [q0, q1, q2], [q2, q3, p3]
Tools
Ruff

132-132: Variable Q0 in function should be lowercase

(N806)


133-133: Variable Q1 in function should be lowercase

(N806)


134-134: Variable Q2 in function should be lowercase

(N806)


138-138: Variable Q3 in function should be lowercase

(N806)


151-164: Remove useless expression.

The expression at the end of the file is not assigned to any variable or used in any way.

- (
-    (
-        (2 + 2j),
-        (1.0956739766575936 + 3.808652046684813j),
-        (3.053667401045204 + 3.5727902021338993j),
-        (5.100619597768561 + 3.326212294969724j),
-    ),
-    (
-        (5.100619597768561 + 3.326212294969724j),
-        (7.580690054131715 + 3.027460529428433j),
-        (10.191347953315187 + 2.7129780700272192j),
-        (8 + 6j),
-    ),
- )
Tools
Ruff

151-164: Found useless expression. Either assign it to a variable or remove it.

(B018)

graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a000bb1 and 48b3a70.

Files selected for processing (7)
  • graphviz2drawio/mx/Curve.py (1 hunks)
  • graphviz2drawio/mx/CurveFactory.py (1 hunks)
  • graphviz2drawio/mx/MxGraph.py (6 hunks)
  • graphviz2drawio/mx/init.py (1 hunks)
  • graphviz2drawio/mx/bezier.py (1 hunks)
  • test/test_curve.py (1 hunks)
  • test/test_graphs.py (1 hunks)
Files skipped from review due to trivial changes (1)
  • graphviz2drawio/mx/init.py
Files skipped from review as they are similar to previous changes (4)
  • graphviz2drawio/mx/MxGraph.py
  • graphviz2drawio/mx/bezier.py
  • test/test_curve.py
  • test/test_graphs.py
Additional context used
Ruff
graphviz2drawio/mx/CurveFactory.py

5-5: Prefer absolute imports over relative imports from parent modules

Replace relative imports from parent modules with absolute imports

(TID252)

Additional comments not posted (8)
graphviz2drawio/mx/CurveFactory.py (3)

11-13: LGTM! Initialization is correct.

The __init__ method correctly initializes the coords attribute.


16-48: LGTM! The method correctly processes SVG paths.

The from_svg method correctly handles SVG path parsing, coordinate translation, and cubic Bézier curve processing.


51-52: LGTM! The function correctly identifies cubic Bézier segments.

The _is_cubic function is straightforward and correctly identifies cubic Bézier segments.

graphviz2drawio/mx/Curve.py (5)

17-29: LGTM! Initialization is correct.

The __init__ method correctly initializes the curve's attributes.


31-33: LGTM! The method correctly returns a string representation.

The __str__ method correctly returns a string representation of the curve.


35-52: LGTM! The method correctly checks for linearity.

The is_linear method correctly checks if a cubic Bézier curve is linear.


55-66: LGTM! The function correctly calculates the slope and y-intercept.

The _line function correctly calculates the slope and y-intercept of a line.


69-76: LGTM! The function correctly reverses the imaginary and real parts.

The _rotate_bezier function correctly reverses the imaginary and real parts for all components of a cubic Bézier curve.

graphviz2drawio/mx/CurveFactory.py Outdated Show resolved Hide resolved
graphviz2drawio/mx/Curve.py Outdated Show resolved Hide resolved
hbmartin and others added 3 commits July 5, 2024 17:22
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@hbmartin hbmartin merged commit 295cb67 into master Jul 6, 2024
5 checks passed
@hbmartin hbmartin deleted the curved branch July 6, 2024 00:31
@hbmartin hbmartin added this to the 0.4 milestone Jul 8, 2024
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.

1 participant