-
Notifications
You must be signed in to change notification settings - Fork 1
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
preliminary str parse and format methods #90
base: feature/edtf-formatter
Are you sure you want to change the base?
Conversation
@coderabbitai full review |
Actions performedFull review triggered. |
WalkthroughThe changes introduced enhance the functionality of the date handling system by expanding type support and improving the parsing and formatting capabilities of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Undate
participant Formatter
User->>Undate: parse(date_string, format)
Undate->>Formatter: get appropriate formatter
Formatter-->>Undate: formatter found
Undate->>Formatter: call parse method
Formatter-->>Undate: return Undate/UndateInterval object
Undate-->>User: return parsed object
sequenceDiagram
participant User
participant Undate
participant Formatter
User->>Undate: format(format)
Undate->>Formatter: get appropriate formatter
Formatter-->>Undate: formatter found
Undate->>Formatter: call to_string method
Formatter-->>Undate: return formatted string
Undate-->>User: return formatted string
Assessment against linked issues
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📋 Review Summary
- Number of files reviewed: 5
- Number of comments: 0
- Number of suggestions: 0
📚 File changes
File path | File changes |
---|---|
src/undate/dateformat/base.py | Added import for Union to support type hinting. |
src/undate/dateformat/edtf/formatter.py | Updated the to_string method to handle both Undate and UndateInterval types. |
src/undate/dateformat/iso8601.py | Updated the to_string method to accept both Undate and UndateInterval types. |
src/undate/undate.py | Added parse and format methods to Undate and UndateInterval classes for handling date string parsing and formatting. |
tests/test_undate.py | Added tests for parsing and formatting methods for Undate and UndateInterval . |
@coderabbitai full review |
Actions performedFull review triggered. |
There was a problem hiding this 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
Files selected for processing (5)
- src/undate/dateformat/base.py (1 hunks)
- src/undate/dateformat/edtf/formatter.py (1 hunks)
- src/undate/dateformat/iso8601.py (1 hunks)
- src/undate/undate.py (2 hunks)
- tests/test_undate.py (2 hunks)
Additional context used
Ruff
src/undate/dateformat/base.py
18-18:
typing.Union
imported but unusedRemove unused import:
typing.Union
(F401)
Additional comments not posted (10)
src/undate/dateformat/base.py (1)
19-19
: Acknowledgment of Python version update.The removal of the comment regarding
functools.cache
suggests that the codebase now targets Python 3.9 or higher, which includes this functionality by default. This is a positive change that simplifies the codebase and leverages newer Python features.src/undate/dateformat/edtf/formatter.py (2)
38-47
: Enhanced flexibility into_string
method.The updated
to_string
method now handles bothUndate
andUndateInterval
types, aligning with the PR's objectives to support flexible date formatting. This change enhances the method's versatility and allows for more comprehensive date handling.
48-48
: Introduction of_undate_to_string
method.The new
_undate_to_string
method encapsulates the logic for converting anUndate
object to a string. This separation of concerns improves code readability and maintainability by delegating the string conversion task to a specialized method.src/undate/dateformat/iso8601.py (2)
52-61
: Enhanced flexibility into_string
method.The updated
to_string
method now handles bothUndate
andUndateInterval
types, aligning with the PR's objectives to support flexible date formatting. This change enhances the method's versatility and allows for more comprehensive date handling.
63-63
: Introduction of_undate_to_string
method.The new
_undate_to_string
method encapsulates the logic for converting anUndate
object to a string. This separation of concerns improves code readability and maintainability by delegating the string conversion task to a specialized method.src/undate/undate.py (2)
177-185
: Review the implementation of theformat
method in theUndate
class.The
format
method is well-implemented with checks for the existence of a formatter class and appropriate error handling for unsupported formats. This method aligns with the PR objectives by allowing flexible formatting ofUndate
objects. However, the comment on line 182 about formatters returning intervals should be addressed to ensure clarity and consistency in the method's behavior.
448-455
: Review the implementation of theformat
method in theUndateInterval
class.This method mirrors the implementation in the
Undate
class, providing consistent functionality across both classes. The method properly checks for the existence of a formatter and raises aValueError
if the format is unsupported, which is good practice. Ensure that all formatters used here are capable of handlingUndateInterval
objects specifically, as the method assumes compatibility.tests/test_undate.py (3)
382-399
: Review thetest_parse
method for theUndate
class.The
test_parse
method adequately tests the parsing functionality with both supported and unsupported formats. It verifies that the correct objects are returned for valid inputs and that appropriate exceptions are raised for unsupported formats. This method effectively ensures that the parsing functionality behaves as expected under various conditions.
400-421
: Review thetest_format
method for theUndate
class.This method tests the formatting functionality thoroughly by checking the output against expected results for both supported formats (EDTF and ISO8601) and handling unsupported formats with exceptions. The inclusion of edge cases like "19XX" and "XXXX-12-31" enhances the robustness of the tests. This method provides good coverage and validation for the formatting functionality.
435-448
: Review thetest_format
method for theUndateInterval
class.The tests for the
UndateInterval
class'sformat
method are comprehensive, covering both typical and open-ended interval cases. The tests ensure that the method correctly formats intervals according to both EDTF and ISO8601 standards and handles unsupported formats appropriately. This thorough testing supports the functionality's robustness and alignment with the PR objectives.
@@ -15,8 +15,8 @@ | |||
import importlib | |||
import logging | |||
import pkgutil | |||
from typing import Dict, Type | |||
from functools import lru_cache # functools.cache not available until 3.9 | |||
from typing import Dict, Type, Union |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import: Union
.
The import of Union
from the typing
module is flagged as unused by the static analysis tool. If it's not needed, consider removing it to keep the code clean and efficient.
-from typing import Dict, Type, Union
+from typing import Dict, Type
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.
from typing import Dict, Type, Union | |
from typing import Dict, Type |
Tools
Ruff
18-18:
typing.Union
imported but unusedRemove unused import:
typing.Union
(F401)
@classmethod | ||
def parse(cls, date_string, format) -> Union["Undate", "UndateInterval"]: | ||
"""parse a string to an undate or undate interval using the specified format; | ||
for now, only supports named formatters""" | ||
formatter_cls = BaseDateFormat.available_formatters().get(format, None) | ||
if formatter_cls: | ||
# NOTE: some parsers may return intervals; is that ok here? | ||
return formatter_cls().parse(date_string) | ||
|
||
raise ValueError(f"Unsupported format '{format}'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review the implementation of the parse
method in the Undate
class.
The parse
method correctly retrieves a formatter class based on the provided format and uses it to parse the date string. However, the method assumes that all formatters will correctly handle the input without further validation or error handling for malformed inputs. Consider adding input validation or more detailed error handling to ensure robustness.
Additionally, the comment on line 172 notes that some parsers may return intervals. It's important to clarify whether this behavior is acceptable or if additional handling is needed when an interval is not expected.
add undate parse and format methods (~ analogous to datetime.strptime and strftime) to allow parsing and formatting dates as strings using any defined formatter class (to be expanded later for more formatting options)
this resolves #87 and is something we can built out for more flexible formatting options later
Summary by CodeRabbit
New Features
Undate
andUndateInterval
classes.Bug Fixes
Tests
Undate
andUndateInterval
classes to validate parsing and formatting functionality across various date formats.