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

Fix TimeStamp and TimeSpan class to use class_from_element method #266

Merged
merged 3 commits into from
Nov 11, 2023

Conversation

cleder
Copy link
Owner

@cleder cleder commented Nov 11, 2023

No description provided.

@cleder cleder linked an issue Nov 11, 2023 that may be closed by this pull request
Copy link

codecov bot commented Nov 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b470a60) 92.82% compared to head (046dd3f) 92.83%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #266   +/-   ##
========================================
  Coverage    92.82%   92.83%           
========================================
  Files           34       34           
  Lines         4810     4812    +2     
========================================
+ Hits          4465     4467    +2     
  Misses         345      345           
Files Coverage Δ
fastkml/base.py 94.73% <100.00%> (ø)
fastkml/kml.py 77.02% <100.00%> (-0.09%) ⬇️
fastkml/times.py 98.11% <100.00%> (+0.11%) ⬆️
fastkml/views.py 91.28% <100.00%> (-0.14%) ⬇️
tests/atom_test.py 100.00% <ø> (ø)
tests/base_test.py 100.00% <ø> (ø)
tests/geometries/point_test.py 100.00% <ø> (ø)
tests/gx_test.py 100.00% <ø> (ø)
tests/times_test.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

if when is not None:
self.timestamp = KmlDateTime.parse(when.text)
kwargs["timestamp"] = KmlDateTime.parse(when.text)

Choose a reason for hiding this comment

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

This file contains at least one console log. Please remove any present.

WatermelonAI Summary

This PR fixes the TimeStamp and TimeSpan class by using the class_from_element method. This improves the business logic by ensuring proper handling and processing of time-related data.

GitHub PRs

Click here to login to Jira
Click here to login to Confluence
Click here to login to Slack
Click here to login to Notion
Click here to login to Linear
Click here to login to Asana
fastkml is an open repo and Watermelon will serve it for free.
🍉🫶
Why not invite more people to your team?

WatermelonAI Summary

This PR fixes the TimeStamp and TimeSpan class to utilize the class_from_element method. It also includes auto fixes from pre-commit.com hooks. These changes align with the business logic of improving the efficiency and accuracy of class implementation.

No results found in GitHub PRs :(

Click here to login to Jira
Click here to login to Confluence
Click here to login to Slack
Click here to login to Notion
Click here to login to Linear
Click here to login to Asana
fastkml is an open repo and Watermelon will serve it for free.
🍉🫶
Try us on any JetBrains IDE!

@@ -177,11 +180,25 @@ def etree_element(
when.text = str(self.timestamp)
return element

def from_element(self, element: Element) -> None:
super().from_element(element)

Choose a reason for hiding this comment

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

This file contains at least one console log. Please remove any present.

Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Some food for thought. View full project report here.

@@ -17,7 +17,7 @@ def print_child_features(element):

k = kml.KML()

with open(fname) as kml_file: # noqa: ENC001
with open(fname) as kml_file:
Copy link
Contributor

Choose a reason for hiding this comment

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

UnicodeDecodeError can occur if the content of the file has characters incompatible with the OS's default encoding. Python uses the OS's default text encoding on the content because encoding is not set. Read more.

Copy link

what-the-diff bot commented Nov 11, 2023

PR Summary

  • Update to class_from_element Method in fastkml/base.py
    The method was modified to include a new parameter - name_spaces, which is also passed to the _get_kwargs method. This enhances how different classes interact with namespaces.

  • Enhancements to from_element Method in fastkml/kml.py
    The method now uses the class_from_element method of TimeSpan and TimeStamp classes, with the name_spaces parameter passed along to enable more customized interactions for time-based features.

  • Updates to TimeStamp and TimeSpan Classes in fastkml/times.py
    The classes were tweaked to include a new parameter - name_spaces. Key methods like __init__ and _get_kwargs now have access to the name_spaces parameter, which will improve the handling of namespaces in these time-related classes.

  • Modification of from_element Method in fastkml/views.py
    The method has been changed to incorporate the class_from_element method of TimeSpan and TimeStamp classes, to which it passes the name_spaces parameter. This update enhances the parsing of time and view-related data.

  • New Test Cases in tests/times_test.py
    Several test cases have been added to vet and validate the functionality of parsing timestamps and timespans under various conditions. These encompass different scenarios, such as parsing timestamps with diverse resolutions, affirming accurate timezone offset, and examining timespans with different resolutions. These tests aim to ensure the robustness and reliability of the new features.

@ghost
Copy link

ghost commented Nov 11, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

id: Optional[str] = None,
target_id: Optional[str] = None,
timestamp: Optional[KmlDateTime] = None,
) -> None:
super().__init__(ns=ns, id=id, target_id=target_id)

Choose a reason for hiding this comment

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

This file contains at least one console log. Please remove any present.

WatermelonAI Summary

This Pull Request aims to fix encoding issues in the KML file reading process. It also updates the TimeStamp and TimeSpan classes to utilize the class_from_element method. Pre-commit.ci was used to automatically fix issues from the pre-commit.com hooks. The changes in this PR will improve the accuracy and reliability of processing KML files in the application.

GitHub PRs

Click here to login to Jira
Click here to login to Confluence
Click here to login to Slack
Click here to login to Notion
Click here to login to Linear
Click here to login to Asana
fastkml is an open repo and Watermelon will serve it for free.
🍉🫶

@cleder cleder merged commit 58041e9 into develop Nov 11, 2023
44 of 48 checks passed
@cleder cleder deleted the 260-refactor-timestamp-and-timespan branch November 11, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor TimeStamp And TimeSpan
1 participant