cleder / fastkml

Fast ๐ŸŒ๏ธ <KML /> processing for python
https://fastkml.readthedocs.io
229 stars 92 forks source link

216 add output verbosity #360

Closed cleder closed 1 month ago

cleder commented 1 month ago

workerB


[!IMPORTANT] Add verbosity control for XML serialization, refactor geometry handling, and update tests in fastkml.

  • Verbosity Control:
    • Introduces Verbosity enum in enums.py with levels terse, normal, and verbose.
    • Updates get_value() in helpers.py to handle verbosity levels for default values.
    • Modifies XML element generation functions in helpers.py to respect verbosity settings.
  • Geometry Classes:
    • Adds verbosity handling in Point, LineString, Polygon, and MultiGeometry classes in geometry.py.
    • Removes extrude and tessellate attributes from _Geometry in geometry.py.
  • Tests:
    • Updates tests in geometry_test.py, linestring_test.py, multigeometry_test.py, point_test.py, polygon_test.py, and gx_test.py to cover verbosity levels.
    • Adds tests for default value handling based on verbosity in geometry_test.py and linestring_test.py.

This description was created by Ellipsis for 69a4c7bb2b5748a10cee830bfce93d49fbe23643. It will automatically update as commits are pushed.


Summary by Sourcery

Add output verbosity control for XML serialization, refactor geometry handling for 'extrude' and 'tessellate' attributes, and update tests to cover new verbosity features and error handling.

New Features:

Enhancements:

Tests:

Summary by CodeRabbit

Release Notes

semanticdiff-com[bot] commented 1 month ago

Review changes with SemanticDiff.

Analyzed 15 of 17 files.

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

Filename Status
:grey_question: pyproject.toml Unsupported file format
:heavy_check_mark: tests/gx_test.py 28.57% smaller
:heavy_check_mark: tests/registry_test.py Analyzed
:grey_question: tests/ogc_conformance/data/kml/Document-places.kml Unsupported file format
:heavy_check_mark: tests/geometries/geometry_test.py 5.21% smaller
:heavy_check_mark: tests/geometries/linestring_test.py Analyzed
:heavy_check_mark: tests/geometries/multigeometry_test.py 0.73% smaller
:heavy_check_mark: tests/geometries/point_test.py Analyzed
:heavy_check_mark: tests/geometries/polygon_test.py Analyzed
:heavy_check_mark: fastkml/base.py Analyzed
:heavy_check_mark: fastkml/enums.py Analyzed
:heavy_check_mark: fastkml/features.py 31.1% smaller
:heavy_check_mark: fastkml/geometry.py 29.4% smaller
:heavy_check_mark: fastkml/gx.py 3.78% smaller
:heavy_check_mark: fastkml/helpers.py 36.67% smaller
:heavy_check_mark: fastkml/kml.py Analyzed
:heavy_check_mark: fastkml/registry.py 43.59% smaller
sourcery-ai[bot] commented 1 month ago

Reviewer's Guide by Sourcery

This pull request implements changes to improve output verbosity control in the fastkml library. It introduces a new Verbosity enum, modifies the RegistryItem class to include a default parameter, and updates various geometry classes to handle verbosity levels when generating XML output. The changes also include refactoring of some geometry classes to remove redundant attributes and improve consistency.

Class diagram for updated geometry classes

classDiagram
    class _Geometry {
        -altitude_mode: Optional[AltitudeMode]
        +altitude_mode: Optional[AltitudeMode]
        +__init__(...)
        +__repr__() -> str
    }
    class Point {
        +extrude: Optional[bool]
        +kml_coordinates: Optional[Coordinates]
        +__init__(...)
        +__repr__() -> str
    }
    class LineString {
        +extrude: Optional[bool]
        +tessellate: Optional[bool]
        +kml_coordinates: Optional[Coordinates]
        +__init__(...)
    }
    class Polygon {
        +extrude: Optional[bool]
        +tessellate: Optional[bool]
        +outer_boundary: Optional[OuterBoundaryIs]
        +inner_boundaries: List[InnerBoundaryIs]
        +__init__(...)
    }
    class MultiGeometry {
        +kml_geometries: List[Union[Point, LineString, Polygon, LinearRing, Self]]
        +__init__(...)
    }
    _Geometry <|-- Point
    _Geometry <|-- LineString
    _Geometry <|-- Polygon
    _BaseObject <|-- MultiGeometry

Class diagram for RegistryItem and Verbosity

classDiagram
    class RegistryItem {
        +ns_ids: Tuple[str, ...]
        +classes: Tuple[type, ...]
        +attr_name: str
        +node_name: str
        +get_kwarg: GetKWArgs
        +set_element: SetElement
        +default: Any
    }
    class Verbosity {
        <<enumeration>>
        +terse
        +normal
        +verbose
    }

File-Level Changes

Change Details Files
Introduced verbosity control for XML output
  • Added a Verbosity enum with terse, normal, and verbose levels
  • Modified RegistryItem class to include a default parameter
  • Updated etree_element function to handle the new default parameter
  • Implemented get_value function to control attribute output based on verbosity
  • Updated various geometry classes to use the new verbosity control
fastkml/enums.py
fastkml/registry.py
fastkml/base.py
fastkml/helpers.py
fastkml/geometry.py
fastkml/features.py
Refactored geometry classes for consistency
  • Removed extrude and tessellate attributes from _Geometry base class
  • Added extrude and tessellate attributes to specific geometry classes where applicable
  • Updated MultiGeometry class to inherit from _BaseObject instead of _Geometry
  • Removed unnecessary parameters from Track and MultiTrack classes
fastkml/geometry.py
fastkml/gx.py
Updated test cases to reflect new verbosity control and refactored classes
  • Added new test cases for verbosity control in various geometry classes
  • Updated existing tests to account for removed attributes in base classes
  • Added tests for new default values in verbose output
tests/geometries/geometry_test.py
tests/geometries/linestring_test.py
tests/geometries/polygon_test.py
tests/geometries/point_test.py
tests/geometries/multigeometry_test.py
tests/gx_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. - **Generate a pull request title:** Write `@sourcery-ai` anywhere in the pull request title to generate a title at any time. - **Generate a pull request summary:** Write `@sourcery-ai summary` anywhere in the pull request body to generate a PR summary at any time. You can also use this command to specify where the summary should be inserted. #### 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).
coderabbitai[bot] commented 1 month ago

Walkthrough

The changes in this pull request include multiple modifications across various files in the fastkml library. Key updates involve the introduction of new parameters, such as default, to several methods, and the renaming of an enum member in fastkml/enums.py. Additionally, several geometry classes have been adjusted to handle attributes like extrude and tessellate more effectively, and new tests have been added to improve coverage and validation of functionality. The pyproject.toml file has also been updated for better dependency management and tool configuration.

Changes

File Path Change Summary
fastkml/base.py Updated etree_element method to include a new default parameter.
fastkml/enums.py Renamed enum member quiet to terse in Verbosity.
fastkml/features.py Added attributes refresh_visibility, fly_to_view, and link to NetworkLink. Set default values for visibility and isopen in _Feature.
fastkml/geometry.py Removed extrude and tessellate from _Geometry, added them to Point, LineString, and Polygon. Updated MultiGeometry to inherit from _BaseObject.
fastkml/gx.py Removed extrude and tessellate parameters from Track and MultiTrack constructors.
fastkml/helpers.py Introduced get_value function for attribute retrieval; updated several existing functions to use get_value.
fastkml/kml.py Updated etree_element method to include a default parameter in xml_subelement_list call.
fastkml/registry.py Updated SetElement protocol to include a default parameter; added default attribute to RegistryItem.
pyproject.toml Updated tool configurations, dependency management, and project metadata.
tests/geometries/geometry_test.py Updated assertions and added new tests focusing on geometry attributes and verbosity handling.
tests/geometries/linestring_test.py Added tests for invalid extrude and tessellate values; updated assertions for to_string method.
tests/geometries/multigeometry_test.py Added tests for MultiGeometry class with verbose settings.
tests/geometries/point_test.py Added tests for to_string method of Point class focusing on extrude attribute.
tests/geometries/polygon_test.py Added tests for Polygon class regarding extrude, tessellate, and altitude_mode parameters.
tests/gx_test.py Removed extrude and tessellate parameters from Track and MultiTrack tests.
tests/registry_test.py Updated set_element function signature to include a default parameter.
tests/ogc_conformance/data/kml/Document-places.kml Modified coordinates elements in KML file to include two decimal places for z-coordinates.

Possibly related PRs

Suggested labels

enhancement, documentation, Configuration changes

Poem

In the fields of KML, we hop and play,
With new defaults to brighten the day.
From quiet to terse, our enums now sing,
As geometries dance, oh what joy they bring!
With tests all updated, our code is so spry,
Hooray for the changes! Let's leap to the sky! ๐Ÿ‡โœจ


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.
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 98.63636% with 3 lines in your changes missing coverage. Please review.

Project coverage is 98.00%. Comparing base (79227e7) to head (69a4c7b). Report is 19 commits behind head on develop.

Files with missing lines Patch % Lines
fastkml/helpers.py 90.90% 0 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #360 +/- ## =========================================== + Coverage 97.93% 98.00% +0.07% =========================================== Files 47 47 Lines 4411 4567 +156 Branches 214 215 +1 =========================================== + Hits 4320 4476 +156 Misses 58 58 Partials 33 33 ```

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

what-the-diff[bot] commented 1 month ago

PR Summary

codiumai-pr-agent-free[bot] commented 1 month ago

PR Reviewer Guide ๐Ÿ”

Here are some key observations to aid the review process:

โฑ๏ธ Estimated effort to review: 4 ๐Ÿ”ต๐Ÿ”ต๐Ÿ”ต๐Ÿ”ตโšช
๐Ÿงช PR contains tests
๐Ÿ”’ No security concerns identified
โšก Recommended focus areas for review

Refactoring
Significant changes to the _Geometry class, including removal of 'extrude' and 'tessellate' attributes. This may affect existing code that relies on these attributes. New Functionality
Introduction of 'get_value' function for handling verbosity levels. This new function needs thorough testing to ensure it correctly handles all verbosity scenarios. Test Coverage
New tests added for verbosity levels in _Geometry class. Ensure these tests cover all possible scenarios, including edge cases.
codiumai-pr-agent-free[bot] commented 1 month ago

CI Failure Feedback ๐Ÿง

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

**Action:** static-tests (3.12)
**Failed stage:** [Typecheck](https://github.com/cleder/fastkml/actions/runs/11192604078/job/31116955358) [โŒ]
**Failed test name:** tests/registry_test.py
**Failure summary:** The action failed due to type errors in the tests/registry_test.py file. Specifically:
  • The argument set_element passed to the RegistryItem function has an incompatible type.
  • The expected type is SetElement, but the provided type is a Callable with a specific signature.
  • This error occurred at multiple lines: 96, 114, 125, 136, 147, 175, 186, 197, 208, 219, 230, 241,
    and 252.
  • A total of 13 errors were found in this file.
  • Relevant error logs: ```yaml 1: ##[group]Operating System 2: Ubuntu ... 296: env: 297: pythonLocation: /opt/hostedtoolcache/Python/3.12.6/x64 298: PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.6/x64/lib/pkgconfig 299: Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.6/x64 300: Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.6/x64 301: Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.6/x64 302: LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.6/x64/lib 303: ##[endgroup] 304: tests/registry_test.py:96: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 305: tests/registry_test.py:114: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 306: tests/registry_test.py:125: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 307: tests/registry_test.py:136: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 308: tests/registry_test.py:147: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 309: tests/registry_test.py:175: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 310: tests/registry_test.py:186: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 311: tests/registry_test.py:197: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 312: tests/registry_test.py:208: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 313: tests/registry_test.py:219: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 314: tests/registry_test.py:230: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 315: tests/registry_test.py:241: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 316: tests/registry_test.py:252: error: Argument "set_element" to "RegistryItem" has incompatible type "Callable[[_XMLObject, NamedArg(Element, 'element'), NamedArg(str, 'attr_name'), NamedArg(str, 'node_name'), NamedArg(int | None, 'precision'), NamedArg(Verbosity | None, 'verbosity')], None]"; expected "SetElement" [arg-type] 317: Found 13 errors in 1 file (checked 50 source files) 318: ##[error]Process completed with exit code 1. ```

    โœจ 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] commented 1 month ago

    Preparing review...

    github-actions[bot] commented 1 month ago

    Preparing review...

    github-actions[bot] commented 1 month ago

    Failed to generate code suggestions for PR

    codiumai-pr-agent-free[bot] commented 1 month ago

    PR Code Suggestions โœจ

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Use a more specific assertion to check for the absence of an attribute ___ **Instead of using assert not hasattr(g, "tessellate"), consider using a more specific
    assertion to check if the tessellate attribute is not present or set to None. This
    will make the test more explicit about what it's checking.** [tests/geometries/geometry_test.py [76]](https://github.com/cleder/fastkml/pull/360/files#diff-c58e16d0b9d54da03108c95fc089fb3cf98368c0a35e0807127fd57d2209fd1fR76-R76) ```diff -assert not hasattr(g, "tessellate") +assert getattr(g, "tessellate", None) is None ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why:
    7
    Add assertions to check for the absence of multiple attributes ___ **Consider adding assertions to check if the extrude attribute is also not present or
    set to None, similar to the tessellate check. This will ensure that both attributes
    are properly handled in the Point class.** [tests/geometries/geometry_test.py [76]](https://github.com/cleder/fastkml/pull/360/files#diff-c58e16d0b9d54da03108c95fc089fb3cf98368c0a35e0807127fd57d2209fd1fR76-R76) ```diff -assert not hasattr(g, "tessellate") +assert getattr(g, "tessellate", None) is None +assert getattr(g, "extrude", None) is None ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why:
    7
    Add default values to optional attributes in class definition ___ **Consider adding type hints for the extrude and tessellate attributes in the
    LineString class. This will improve code readability and help with static type
    checking.** [fastkml/geometry.py [549-551]](https://github.com/cleder/fastkml/pull/360/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR549-R551) ```diff -extrude: Optional[bool] -tessellate: Optional[bool] -kml_coordinates: Optional[Coordinates] +extrude: Optional[bool] = None +tessellate: Optional[bool] = None +kml_coordinates: Optional[Coordinates] = None ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why:
    7
    Expand the class docstring to provide more detailed information ___ **Consider adding a docstring to the MultiGeometry class to explain its purpose and
    usage. This will improve code documentation and make it easier for other developers
    to understand and use the class.** [fastkml/geometry.py [1336-1337]](https://github.com/cleder/fastkml/pull/360/files#diff-47314ad0817fc0fdde5a4ccab1250d104c27d592f1dd0f9952b454a4e3cdf65dR1336-R1337) ```diff class MultiGeometry(_BaseObject): - """A container for zero or more geometry primitives.""" + """ + A container for zero or more geometry primitives. + This class represents a collection of KML geometries, which can include + Points, LineStrings, Polygons, LinearRings, and other MultiGeometries. + It allows for the grouping and manipulation of multiple geometric objects + as a single entity within a KML document. + """ + ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why:
    7
    Use parameterized tests to reduce code duplication and improve test maintainability ___ **Consider using parameterized tests for the test_to_string_* methods. This would
    reduce code duplication and make it easier to add new test cases in the future.** [tests/geometries/point_test.py [60-103]](https://github.com/cleder/fastkml/pull/360/files#diff-68b57f69d1b92ba994d69ef07c436d7e56a73eeb9e800d7d95b22758d8231153R60-R103) ```diff -def test_to_string_terse_default(self) -> None: - """Test the to_string method, exclude default for extrude in terse mode.""" +import pytest + +@pytest.mark.parametrize("verbosity, extrude, expected_extrude", [ + (Verbosity.terse, False, ""), + (Verbosity.terse, True, "extrude>1010" in result + if expected_extrude: + assert expected_extrude in result + else: + assert "extrude" not in result - point = Point(geometry=p, extrude=False) - - assert "coordinates>" in point.to_string(verbosity=Verbosity.terse) - assert "extrude" not in point.to_string(verbosity=Verbosity.terse) - -def test_to_string_terse_non_default(self) -> None: - """Test the to_string method, include extrude when true in terse mode.""" - p = geo.Point(1, 2) - - point = Point(geometry=p, extrude=True) - - assert "coordinates>" in point.to_string(verbosity=Verbosity.terse) - assert "extrude>1 None: - """Test the to_string method, include default for extrude in verbose mode.""" - p = geo.Point(1, 2) - - point = Point(geometry=p, extrude=False) - - assert "coordinates>" in point.to_string(verbosity=Verbosity.verbose) - assert "extrude>0 None: - """Test the to_string method, include extrude when true in verbose mode.""" - p = geo.Point(1, 2) - - point = Point(geometry=p, extrude=True) - - assert "coordinates>" in point.to_string(verbosity=Verbosity.verbose) - assert "extrude>1 None: - """Test the to_string method, include extrude when true in verbose mode.""" - p = geo.Point(1, 2) - - point = Point(geometry=p, extrude=False) - - assert "coordinates>" in point.to_string(verbosity=Verbosity.verbose) - assert "extrude>0
    Suggestion importance[1-10]: 7 Why:
    7
    Use a context manager for asserting XML element counts to improve test readability and maintainability ___ **Consider using a context manager for asserting the count of specific XML elements in
    the test_multi_geometries_verbose method. This would make the test more readable and
    easier to maintain.** [tests/geometries/polygon_test.py [265-289]](https://github.com/cleder/fastkml/pull/360/files#diff-0ef5481c38be48c9edb668e71b4f2724eb62fef20501b4f46f0f4508db55ecf1R265-R289) ```diff +from contextlib import contextmanager + +@contextmanager +def assert_xml_element_count(xml, element, expected_count): + actual_count = xml.count(element) + yield + assert actual_count == expected_count, f"Expected {expected_count} occurrences of '{element}', but found {actual_count}" + def test_multi_geometries_verbose(self) -> None: - p = geo.Point(1, 2) - ls = geo.LineString(((1, 2), (2, 0))) - lr = geo.LinearRing(((0, 0), (0, 1), (1, 1), (1, 0), (0, 0))) - poly = geo.Polygon( - [(0, 0), (0, 1), (1, 1), (1, 0), (0, 0)], - [[(0.1, 0.1), (0.1, 0.9), (0.9, 0.9), (0.9, 0.1), (0.1, 0.1)]], - ) - gc = geo.GeometryCollection([p, ls, lr, poly]) - mp = geo.MultiPolygon( - [ - ( - ((0.0, 0.0), (0.0, 1.0), (1.0, 1.0), (1.0, 0.0)), - [((0.1, 0.1), (0.1, 0.2), (0.2, 0.2), (0.2, 0.1))], - ), - (((0.0, 0.0), (0.0, 2.0), (1.0, 1.0), (1.0, 0.0)),), - ], - ) - ml = geo.MultiLineString([[(1, 2), (3, 4)], [(5, 6), (7, 8)]]) - mgc = geo.GeometryCollection([gc, mp, ml]) - mg = MultiGeometry(ns="", geometry=mgc) + # ... (existing setup code) xml = mg.to_string(verbosity=Verbosity.verbose) - assert xml.count("tessellate>0<") == 12 # points do not have tessellate - assert xml.count("extrude>0<") == 13 - assert xml.count("altitudeMode>clampToGround<") == 13 + with assert_xml_element_count(xml, "tessellate>0<", 12): + pass # points do not have tessellate + with assert_xml_element_count(xml, "extrude>0<", 13): + pass + with assert_xml_element_count(xml, "altitudeMode>clampToGround<", 13): + pass ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why:
    7
    Use a parameterized test to reduce code duplication and improve test maintainability for LineString to_string methods ___ **Consider using a parameterized test for the test_to_string_* methods to reduce code
    duplication and improve test maintainability.** [tests/geometries/linestring_test.py [178-221]](https://github.com/cleder/fastkml/pull/360/files#diff-45bb62f1478c8e27bd84d343f667c902609abe57de39b34600795afe3acf4a22R178-R221) ```diff -def test_to_string_terse_default(self) -> None: +import pytest + +@pytest.mark.parametrize("verbosity, extrude, tessellate, expected_tessellate, expected_extrude", [ + (Verbosity.terse, False, False, "", ""), + (Verbosity.terse, True, True, "tessellate>11001100 None: - ls = geo.LineString(((1, 2), (2, 0))) - line_string = LineString(geometry=ls, extrude=True, tessellate=True) + if expected_extrude: + assert expected_extrude in xml + else: + assert "extrude" not in xml - xml = line_string.to_string(verbosity=Verbosity.terse) - - assert "tessellate>11 None: - ls = geo.LineString(((1, 2), (2, 0))) - line_string = LineString(geometry=ls, extrude=False, tessellate=False) - - xml = line_string.to_string(verbosity=Verbosity.verbose) - - assert "tessellate>00 None: - ls = geo.LineString(((1, 2), (2, 0))) - line_string = LineString(geometry=ls, extrude=True, tessellate=True) - - xml = line_string.to_string(verbosity=Verbosity.verbose) - - assert "tessellate>11 None: - ls = geo.LineString(((1, 2), (2, 0))) - line_string = LineString(geometry=ls) - - xml = line_string.to_string(verbosity=Verbosity.verbose) - - assert "tessellate>00
    Suggestion importance[1-10]: 7 Why:
    7

    ๐Ÿ’ก Need additional feedback ? start a PR chat

    github-actions[bot] commented 1 month ago

    Preparing review...

    github-actions[bot] commented 1 month ago

    Preparing review...