Fix inner boundaries for polygon #355

[cleder]

User description

---

PR Type

Bug fix, Tests

---

Description

• Refactored the InnerBoundaryIs class to use a single kml_geometry instead of a list, simplifying the handling of inner boundaries.
• Updated the Polygon class to manage multiple InnerBoundaryIs instances, ensuring correct handling of interior rings.
• Modified test cases to align with the refactored class structures, ensuring comprehensive coverage of new logic.
• Corrected test method names and added assertions to improve test accuracy and reliability.

---

Changes walkthrough 📝

	Relevant files
Bug fix	geometry.py Refactor inner boundary handling in geometry classes          fastkml/geometry.py Changed InnerBoundaryIs class to use a single kml_geometry instead of a list. Updated Polygon class to handle multiple InnerBoundaryIs instances. Modified logic to raise GeometryError for mutual exclusivity. Updated property methods to reflect changes in data structure. +53/-55
Tests	boundaries_test.py Update tests for inner boundary refactor                                  tests/geometries/boundaries_test.py Updated tests to reflect changes in InnerBoundaryIs class. Added test for handling multiple LinearRing elements. +12/-16  geometry_test.py Correct test names and add geometry assertions                      tests/geometries/geometry_test.py Corrected test method name from test_tesselate to test_tessellate. Added assertions to ensure geometry is not None. +4/-2      polygon_test.py Update polygon tests for refactored boundary handling        tests/geometries/polygon_test.py - Updated test assertions to reflect changes in `Polygon` class. +1/-1      repr_eq_test.py Adjust representation tests for boundary refactor                tests/repr_eq_test.py Updated TestRepr class to reflect changes in boundary attributes. Adjusted test data to use new inner_boundaries structure. +16/-19

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

Summary by Sourcery

Refactor the handling of inner boundaries in the geometry classes by updating the InnerBoundaryIs class to use a single kml_geometry and modifying the Polygon class to manage multiple InnerBoundaryIs instances. Update test cases to reflect these changes and improve test accuracy.

Bug Fixes:

• Fix handling of inner boundaries in the Polygon class by updating the InnerBoundaryIs class to use a single kml_geometry instead of a list, ensuring correct management of interior rings.

Enhancements:

• Refactor the InnerBoundaryIs class to simplify the handling of inner boundaries by using a single kml_geometry.
• Update the Polygon class to manage multiple InnerBoundaryIs instances, improving the handling of interior rings.

Tests:

• Modify test cases to align with the refactored InnerBoundaryIs and Polygon classes, ensuring comprehensive coverage of the new logic.
• Correct test method names and add assertions to improve test accuracy and reliability.

Summary by CodeRabbit

• New Features

  • Streamlined handling of KML geometries with updated attributes for boundaries.
  • Enhanced clarity in the naming conventions for boundary attributes.

• Bug Fixes

  • Corrected method names and assertions in tests to ensure proper functionality.
  • Improved robustness of tests by validating geometry instantiation before assertions.

• Documentation

  • Updated test descriptions and docstrings for clarity on boundary handling.

---

[!IMPORTANT] Refactor inner boundary handling in fastkml by updating InnerBoundaryIs and Polygon classes, and adjust tests accordingly.

• Behavior:
  • Refactor InnerBoundaryIs to use a single kml_geometry instead of a list in geometry.py.
  • Update Polygon to handle multiple InnerBoundaryIs instances, ensuring correct interior ring handling.
  • Raise GeometryError for mutual exclusivity in geometry.py.
• Tests:
  • Update tests in boundaries_test.py to reflect changes in InnerBoundaryIs.
  • Add test for multiple LinearRing elements in boundaries_test.py.
  • Correct test method name from test_tesselate to test_tessellate in geometry_test.py.
  • Add assertions for geometry existence in geometry_test.py.
  • Update test assertions in polygon_test.py for Polygon class changes.
  • Adjust representation tests in repr_eq_test.py for boundary refactor.

^{This description was created by for fd61d0b27e91dcdcb587013a166f83544b0b7e2a. It will automatically update as commits are pushed.}

[semanticdiff-com[bot]]

Review changes with SemanticDiff.

Analyzed 5 of 5 files.

Overall, the semantic diff is 35% smaller than the GitHub diff.

	Filename	Status
:heavy_check_mark:	tests/repr_eq_test.py	46.51% smaller
:heavy_check_mark:	tests/geometries/boundaries_test.py	30.71% smaller
:heavy_check_mark:	tests/geometries/geometry_test.py	39.9% smaller
:heavy_check_mark:	tests/geometries/polygon_test.py	38.85% smaller
:heavy_check_mark:	fastkml/geometry.py	33.07% smaller

[coderabbitai[bot]]

[!WARNING]

Rate limit exceeded

@cleder has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 28 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](https://coderabbit.ai/docs/faq) for further information.

📥 Commits

Files that changed from the base of the PR and between 118c06427d1e82dd47a92803999af0b9cd9442f8 and fd61d0b27e91dcdcb587013a166f83544b0b7e2a.

Walkthrough

The changes primarily involve significant modifications to the InnerBoundaryIs and Polygon classes in the fastkml/geometry.py file. The InnerBoundaryIs class has been simplified to handle a single geometry instead of multiple, with corresponding updates to constructor parameters and attributes. The Polygon class reflects similar changes, renaming attributes and adjusting constructor parameters to accommodate a single outer boundary and a list of inner boundaries. Additionally, various test files have been updated to align with these structural changes, ensuring consistency in testing the new implementations.

Changes

File	Change Summary
fastkml/geometry.py	- InnerBoundaryIs: Changed kml_geometries to kml_geometry, updated constructor to accept a single geometry. Removed logic for multiple geometries. - Polygon: Renamed outer_boundary_is to outer_boundary and inner_boundary_is to inner_boundaries, updated constructor and logic accordingly.
tests/geometries/boundaries_test.py	- Updated TestBoundaries: Changed InnerBoundaryIs initialization to accept a single geometry. Renamed test_read_inner_boundary to test_read_inner_boundary_multiple_linestrings.
tests/geometries/polygon_test.py	- Updated test_empty_polygon: Renamed outer_boundary_is to outer_boundary in assertions. Minor formatting adjustments made.

Possibly related PRs

• 322: The changes in the InnerBoundaryIs class in the main PR directly relate to the modifications made in the same class within the retrieved PR, which also focuses on enhancing the boundary management in the Polygon class.

Suggested labels

bug_fix, enhancement, Review effort [1-5]: 4

Poem

🐇 In the fields where boundaries lie,
A single shape now meets the eye.
No more a crowd of lines to see,
Just one clear path, as it should be!
With tests aligned and names so bright,
KML's geometry shines with light! 🌟

---

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

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

🪧 Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit , please review it.` - `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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@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 using 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. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

[pep8speaks]

Hello @cleder! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file tests/repr_eq_test.py:

Line 480:90: E501 line too long (92 > 89 characters) Line 578:90: E501 line too long (92 > 89 characters) Line 676:90: E501 line too long (92 > 89 characters) Line 919:90: E501 line too long (92 > 89 characters) Line 1006:90: E501 line too long (90 > 89 characters) Line 1008:90: E501 line too long (98 > 89 characters) Line 1009:90: E501 line too long (96 > 89 characters) Line 1010:90: E501 line too long (100 > 89 characters) Line 1012:90: E501 line too long (93 > 89 characters) Line 1796:90: E501 line too long (377 > 89 characters)

Comment last updated at 2024-10-02 16:51:30 UTC

[sourcery-ai[bot]]

Reviewer's Guide by Sourcery

This pull request refactors the handling of inner boundaries for polygons in the fastkml library. The main changes involve simplifying the InnerBoundaryIs class to use a single kml_geometry instead of a list, updating the Polygon class to manage multiple InnerBoundaryIs instances, and modifying test cases to align with the new class structures. These changes aim to improve the handling of interior rings in polygons and enhance the overall reliability of the library.

Updated class diagram for InnerBoundaryIs and Polygon classes

classDiagram
    class InnerBoundaryIs {
        - LinearRing kml_geometry
        - Optional~str~ ns
        - Optional~Dict~ name_spaces
        - Optional~geo.LinearRing~ geometry
        + __init__(ns, name_spaces, geometry, kml_geometry)
        + bool __bool__()
        + str __repr__()
        + Optional~geo.LinearRing~ geometry()
    }

    class Polygon {
        - Optional~OuterBoundaryIs~ outer_boundary
        - Optional~List~ inner_boundaries
        - Optional~bool~ extrude
        - Optional~bool~ tessellate
        - Optional~AltitudeMode~ altitude_mode
        - Optional~geo.Polygon~ geometry
        + __init__(ns, name_spaces, extrude, tessellate, altitude_mode, outer_boundary, inner_boundaries, geometry)
        + bool __bool__()
        + Optional~geo.Polygon~ geometry()
        + str __repr__()
    }

    InnerBoundaryIs --> LinearRing
    Polygon --> OuterBoundaryIs
    Polygon --> InnerBoundaryIs

File-Level Changes

Change	Details	Files
Refactored InnerBoundaryIs class to use a single kml_geometry	Changed kml_geometries attribute to kml_geometry Updated init method to handle a single geometry instead of multiple Modified property methods to reflect the new single geometry structure	fastkml/geometry.py
Updated Polygon class to handle multiple InnerBoundaryIs instances	Changed inner_boundary_is attribute to inner_boundaries Modified init method to handle a list of InnerBoundaryIs instances Updated geometry property to work with the new inner_boundaries structure	fastkml/geometry.py
Modified test cases to align with refactored class structures	Updated assertions in test_inner_boundary method Added test for handling multiple LinearRing elements in InnerBoundaryIs Adjusted test data to use new inner_boundaries structure	tests/geometries/boundaries_test.py tests/repr_eq_test.py
Improved test accuracy and reliability	Corrected test method name from test_tesselate to test_tessellate Added assertions to ensure geometry is not None in MultiGeometry and Polygon tests	tests/geometries/geometry_test.py tests/geometries/polygon_test.py

---

Tips and commands

#### Interacting with Sourcery - **Trigger a new review:** Comment `@sourcery-ai review` on the pull request. - **Continue discussions:** Reply directly to Sourcery's review comments. - **Generate a GitHub issue from a review comment:** Ask Sourcery to create an issue from a review comment by replying to it. #### Customizing Your Experience Access your [dashboard](https://app.sourcery.ai) to: - Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others. - Change the review language. - Add, remove or edit custom review instructions. - Adjust other review settings. #### Getting Help - [Contact our support team](mailto:support@sourcery.ai) for questions or feedback. - Visit our [documentation](https://docs.sourcery.ai) for detailed guides and information. - Keep in touch with the Sourcery team by following us on [X/Twitter](https://x.com/SourceryAI), [LinkedIn](https://www.linkedin.com/company/sourcery-ai/) or [GitHub](https://github.com/sourcery-ai).

[codiumai-pr-agent-free[bot]]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Code Refactoring The InnerBoundaryIs class has been refactored to use a single kml_geometry instead of a list. This change might affect how multiple inner boundaries are handled. Logic Change The Polygon class has been updated to handle multiple InnerBoundaryIs instances. This change might affect how polygons with multiple inner boundaries are processed. Test Coverage A new test case has been added for handling multiple LinearRing elements in InnerBoundaryIs. Ensure this test adequately covers the new functionality.

[github-actions[bot]]

Failed to generate code suggestions for PR

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 93.54839% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.93%. Comparing base (181e2fe) to head (fd61d0b). Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
fastkml/geometry.py	89.47%	0 Missing and 2 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #356 +/- ## =========================================== + Coverage 97.88% 97.93% +0.05% =========================================== Files 47 47 Lines 4395 4411 +16 Branches 216 214 -2 =========================================== + Hits 4302 4320 +18 + Misses 59 58 -1 + Partials 34 33 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[github-actions[bot]]

Preparing review...

[github-actions[bot]]

Preparing review...

[codiumai-pr-agent-free[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Improve variable naming for better code readability ___ **Consider using a more descriptive variable name instead of lr in the list comprehension. For example, linear_ring would be clearer and more self-explanatory.** [fastkml/geometry.py [956-958]](https://github.com/cleder/fastkml/pull/356/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR956-R958) ```diff if kml_geometry is None: kml_geometry = LinearRing(ns=ns, name_spaces=name_spaces, geometry=geometry) +self.kml_geometry = kml_geometry ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7
Possible issue	Add type checking for input parameters to prevent potential runtime errors ___ **Consider adding a type check for the geometry parameter to ensure it's a geo.LinearRing object before creating the LinearRing instance. This can help prevent potential runtime errors.** [fastkml/geometry.py [956-958]](https://github.com/cleder/fastkml/pull/356/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR956-R958) ```diff if kml_geometry is None: + if not isinstance(geometry, geo.LinearRing): + raise TypeError("geometry must be a geo.LinearRing object") kml_geometry = LinearRing(ns=ns, name_spaces=name_spaces, geometry=geometry) self.kml_geometry = kml_geometry ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7
Enhancement	Use list comprehension for creating inner boundaries to improve code conciseness ___ **Consider using a list comprehension instead of a for loop when creating the inner_boundaries list. This can make the code more concise and potentially more efficient.** [fastkml/geometry.py [1096-1099]](https://github.com/cleder/fastkml/pull/356/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR1096-R1099) ```diff if geometry is not None: outer_boundary = OuterBoundaryIs(geometry=geometry.exterior) - inner_boundaries = [ - InnerBoundaryIs(geometry=interior) for interior in geometry.interiors - ] + inner_boundaries = [InnerBoundaryIs(geometry=interior) for interior in geometry.interiors] ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7
Possible bug	Add null checks to prevent potential AttributeErrors when accessing nested attributes ___ **Consider adding a check to ensure that self.outer_boundary.geometry is not None before using it in the geo.Polygon.from_linear_rings() method. This can help prevent potential AttributeErrors if self.outer_boundary or its geometry is None.** [fastkml/geometry.py [1138-1141]](https://github.com/cleder/fastkml/pull/356/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR1138-R1141) ```diff if not self.inner_boundaries: - return geo.Polygon.from_linear_rings( - cast(geo.LinearRing, self.outer_boundary.geometry), - ) + outer_geometry = self.outer_boundary.geometry if self.outer_boundary else None + if outer_geometry is None: + return None + return geo.Polygon.from_linear_rings(cast(geo.LinearRing, outer_geometry)) ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why:	7

💡 Need additional feedback ? start a PR chat

[codiumai-pr-agent-free[bot]]

CI Failure Feedback 🧐

(Checks updated until commit https://github.com/cleder/fastkml/commit/118c06427d1e82dd47a92803999af0b9cd9442f8)

**Action:** pypy (pypy-3.8)

**Failed stage:** [Test with pytest](https://github.com/cleder/fastkml/actions/runs/11147957797/job/30983495281) [❌]

Relevant error logs: ```yaml 1: ##[group]Operating System 2: Ubuntu ... 225: Python3_ROOT_DIR: /opt/hostedtoolcache/PyPy/3.8.16/x64 226: PKG_CONFIG_PATH: /opt/hostedtoolcache/PyPy/3.8.16/x64/bin/lib/pkgconfig 227: ##[endgroup] 228: ============================= test session starts ============================== 229: platform linux -- Python 3.8.16[pypy-7.3.11-final], pytest-8.3.3, pluggy-1.5.0 230: rootdir: /home/runner/work/fastkml/fastkml 231: configfile: pyproject.toml 232: plugins: cov-5.0.0 233: collected 508 items / 1 error 234: ==================================== ERRORS ==================================== 235: _____________ ERROR collecting tests/geometries/boundaries_test.py _____________ ... 251: source_stat, co = _rewrite_test(fn, self.config) 252: /opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/site-packages/_pytest/assertion/rewrite.py:355: in _rewrite_test 253: tree = ast.parse(source, filename=strfn) 254: /opt/hostedtoolcache/PyPy/3.8.16/x64/lib/pypy3.8/ast.py:47: in parse 255: return compile(source, filename, mode, flags, 256: E File "/home/runner/work/fastkml/fastkml/tests/geometries/boundaries_test.py", line 75 257: E assert inner_boundary.to_string(prettyprint=False).strip() == ( 258: E ^ 259: E IndentationError: unexpected indent 260: =============================== warnings summary =============================== 261: fastkml/config.py:38 262: /home/runner/work/fastkml/fastkml/fastkml/config.py:38: UserWarning: Package `lxml` missing. Pretty print will be disabled 263: warnings.warn("Package `lxml` missing. Pretty print will be disabled") # noqa: B028 264: -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html 265: =========================== short test summary info ============================ 266: ERROR tests/geometries/boundaries_test.py 267: !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!! 268: ========================= 1 warning, 1 error in 3.34s ========================== 269: ##[error]Process completed with exit code 2. ```

---

✨ CI feedback usage guide:

---

The CI feedback tool (`/checks)` automatically triggers when a PR has a failed check. The tool analyzes the failed checks and provides several feedbacks: - Failed stage - Failed test name - Failure summary - Relevant error logs In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: ``` /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}" ``` where `{repo_name}` is the name of the repository, `{run_number}` is the run number of the failed check, and `{job_number}` is the job number of the failed check. #### Configuration options - `enable_auto_checks_feedback` - if set to true, the tool will automatically provide feedback when a check is failed. Default is true. - `excluded_checks_list` - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list. - `enable_help_text` - if set to true, the tool will provide a help message with the feedback. Default is true. - `persistent_comment` - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true. - `final_update_message` - if `persistent_comment` is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true. See more information about the `checks` tool in the [docs](https://pr-agent-docs.codium.ai/tools/ci_feedback/).

[github-actions[bot]]

Preparing review...

[github-actions[bot]]

Preparing review...