Skip to content

Conversation

JarbasAl
Copy link
Member

@JarbasAl JarbasAl commented Nov 7, 2024

Summary by CodeRabbit

  • New Features

    • Introduced automated workflows for running formatter and unit tests in GitHub Actions.
    • Added comprehensive unit tests for date and time formatting functions across multiple languages, enhancing localization support.
    • Implemented extraction tests for durations and datetime values in various languages, ensuring accurate parsing.
  • Bug Fixes

    • Improved handling of edge cases and ambiguous time expressions in datetime extraction functions for several languages.
  • Documentation

    • Updated test files to include detailed comments and structure for better understanding and future enhancements.

Copy link
Contributor

coderabbitai bot commented Nov 7, 2024

Warning

Rate limit exceeded

@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 21 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

Reviewing files that changed from the base of the PR and between 79732fe and fe42c06.

Walkthrough

This pull request introduces multiple new GitHub Actions workflows and a comprehensive suite of unit tests for various date and time extraction functionalities in the ovos_date_parser library. The workflows automate the running of formatter and unit tests across different Python versions and are triggered by pull requests and manual dispatch. The new test files cover multiple languages, ensuring correct parsing and formatting of dates and durations, with specific attention to localization requirements.

Changes

File Path Change Summary
.github/workflows/fmtunit_tests.yml New workflow file to automate formatter unit tests, triggered on pull requests and pushes to master, with jobs for Python build tests and unit tests across multiple Python versions.
.github/workflows/unit_tests.yml New workflow file for parser component unit tests, similar structure to fmtunit_tests.yml, with jobs for Python build tests and unit tests across multiple Python versions.
test/format_tests/test_format.py New test suite for date formatting in English, with classes and methods validating various date formatting functions.
test/format_tests/test_format_az.py New test suite for Azerbaijani date formatting, validating multiple formatting functions.
test/format_tests/test_format_ca.py New test suite for Catalan date formatting, focused on the nice_time function.
test/format_tests/test_format_cs.py New test suite for Czech date formatting, validating multiple date and time functions.
test/format_tests/test_format_da.py New test suite for Danish date formatting, focused on the nice_time function.
test/format_tests/test_format_de.py New test suite for German date formatting, validating multiple date and time functions.
test/format_tests/test_format_en.py New test suite for English date formatting, validating the nice_time and nice_duration functions.
test/format_tests/test_format_es.py New test suite for Spanish date formatting, validating the nice_time function.
test/format_tests/test_format_eu.py New test suite for Basque date formatting, validating the nice_time function.
test/format_tests/test_format_fa.py New test suite for Persian date formatting, validating the nice_time function.
test/format_tests/test_format_fr.py New test suite for French date formatting, validating multiple date and time functions.
test/format_tests/test_format_hu.py New test suite for Hungarian date formatting, focused on the nice_time function.
test/format_tests/test_format_it.py New test suite for Italian date formatting, validating the nice_time function.
test/format_tests/test_format_nl.py New test suite for Dutch date formatting, validating the nice_time function.
test/format_tests/test_format_pl.py New test suite for Polish date formatting, validating both nice_time and nice_duration functions.
test/format_tests/test_format_pt.py New test suite for Portuguese date formatting, validating multiple date and duration functions.
test/format_tests/test_format_ru.py New test suite for Russian date formatting, validating multiple date and time functions.
test/format_tests/test_format_sl.py New test suite for Slovenian date formatting, validating multiple date and time functions.
test/format_tests/test_format_sv.py New test suite for Swedish date formatting, validating multiple date and time functions.
test/format_tests/test_format_uk.py New test suite for Ukrainian date formatting, validating multiple date and time functions.
test/parse_tests/test_parse_az.py New test suite for Azerbaijani duration and datetime extraction functions.
test/parse_tests/test_parse_ca.py New test suite for Catalan datetime extraction functions.
test/parse_tests/test_parse_cs.py New test suite for Czech duration and datetime extraction functions.
test/parse_tests/test_parse_da.py New test suite for Danish datetime extraction functions.
test/parse_tests/test_parse_datetime_uk.py New test suite for Ukrainian duration and datetime extraction functions.
test/parse_tests/test_parse_de.py New test suite for German duration and datetime extraction functions.
test/parse_tests/test_parse_en.py New test suite for English duration and datetime extraction functions.
test/parse_tests/test_parse_es.py New test suite for Spanish duration and datetime extraction functions.
test/parse_tests/test_parse_eu.py New test suite for Basque datetime extraction functions.
test/parse_tests/test_parse_fa.py New test suite for Persian duration and datetime extraction functions.
test/parse_tests/test_parse_fr.py New test suite for French duration and datetime extraction functions.
test/parse_tests/test_parse_it.py New test suite for Italian datetime extraction functions.
test/parse_tests/test_parse_nl.py New test suite for Dutch datetime extraction functions.
test/parse_tests/test_parse_pl.py New test suite for Polish datetime extraction functions.
test/parse_tests/test_parse_pt.py New test suite for Portuguese datetime extraction functions.
test/parse_tests/test_parse_ru.py New test suite for Russian duration and datetime extraction functions.
test/parse_tests/test_parse_sv.py New test suite for Swedish duration and datetime extraction functions.

Poem

In the garden of time, we hop and play,
With formats and functions, we brighten the day.
From Azerbaijani to Swedish, we test with glee,
Each rabbit's a coder, as happy as can be! 🐇✨
So let’s celebrate changes, both big and small,
For in the world of dates, we’ll conquer them all!


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

CodeRabbit Configuration 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.

@github-actions github-actions bot added feature and removed feature labels Nov 7, 2024
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: 37

🧹 Outside diff range and nitpick comments (119)
.github/workflows/fmtunit_tests.yml (2)

6-30: Consider using YAML anchors to reduce duplication.

The paths-ignore configuration is identical for both triggers. Consider using YAML anchors to maintain DRY principles:

name: Run Formatter UnitTests
on:
+ ignore_paths: &ignore_paths
+   - 'ovos_date_parser/version.py'
+   - 'requirements/**'
+   - 'examples/**'
+   - '.github/**'
+   - '.gitignore'
+   - 'LICENSE'
+   - 'CHANGELOG.md'
+   - 'MANIFEST.in'
+   - 'README.md'
+   - 'scripts/**'
  pull_request:
    branches:
      - dev
    paths-ignore:
-     - 'ovos_date_parser/version.py'
-     - 'requirements/**'
-     - 'examples/**'
-     - '.github/**'
-     - '.gitignore'
-     - 'LICENSE'
-     - 'CHANGELOG.md'
-     - 'MANIFEST.in'
-     - 'README.md'
-     - 'scripts/**'
+     *ignore_paths
  push:
    branches:
      - master
    paths-ignore:
-     - 'ovos_date_parser/version.py'
-     - 'requirements/**'
-     - 'examples/**'
-     - '.github/**'
-     - '.gitignore'
-     - 'LICENSE'
-     - 'CHANGELOG.md'
-     - 'MANIFEST.in'
-     - 'README.md'
-     - 'scripts/**'
+     *ignore_paths

63-76: Add newline at EOF and coverage configuration looks good.

The coverage configuration is well-structured with appropriate token usage and flags. However, add a newline at the end of the file to comply with YAML standards.

           flags: fmttests
           name: codecov-unittests
-          verbose: true
+          verbose: true
+
🧰 Tools
🪛 yamllint

[error] 76-76: no new line character at the end of file

(new-line-at-end-of-file)

.github/workflows/unit_tests.yml (5)

2-31: Refactor duplicated paths-ignore configuration.

The paths-ignore configuration is duplicated between pull_request and push triggers. Consider using YAML anchors to make it more maintainable.

name: Run Parser UnitTests
on:
+ paths-ignore: &ignore_paths
+   - 'ovos_date_parser/version.py'
+   - 'requirements/**'
+   - 'examples/**'
+   - '.github/**'
+   - '.gitignore'
+   - 'LICENSE'
+   - 'CHANGELOG.md'
+   - 'MANIFEST.in'
+   - 'README.md'
+   - 'scripts/**'
  pull_request:
    branches:
      - dev
-   paths-ignore:
-     - 'ovos_date_parser/version.py'
-     - 'requirements/**'
-     - 'examples/**'
-     - '.github/**'
-     - '.gitignore'
-     - 'LICENSE'
-     - 'CHANGELOG.md'
-     - 'MANIFEST.in'
-     - 'README.md'
-     - 'scripts/**'
+   paths-ignore: *ignore_paths
  push:
    branches:
      - master
-   paths-ignore:
-     - 'ovos_date_parser/version.py'
-     - 'requirements/**'
-     - 'examples/**'
-     - '.github/**'
-     - '.gitignore'
-     - 'LICENSE'
-     - 'CHANGELOG.md'
-     - 'MANIFEST.in'
-     - 'README.md'
-     - 'scripts/**'
+   paths-ignore: *ignore_paths

34-38: Consider using a newer Python version for build tests.

The build tests are using Python 3.8. Consider using a newer version (e.g., 3.11) since it's already supported in the unit tests matrix.


39-43: Consider removing Python 3.7 from the test matrix.

Python 3.7 reached end-of-life in June 2023. Consider removing it from the test matrix unless backward compatibility is specifically required.


50-62: Optimize dependency installation steps.

Consider combining the dependency installation steps and using pip's cache feature to speed up the workflow.

  - name: Install System Dependencies
    run: |
      sudo apt-get update
      sudo apt install python3-dev libssl-dev
      python -m pip install build wheel
+     python -m pip install --upgrade pip
  - name: Install test dependencies
    run: |
-     sudo apt install libssl-dev
      pip install pytest pytest-timeout pytest-cov
-  - name: Install repo
-    run: |
      pip install git+https://github.com/OpenVoiceOS/ovos-number-parser
      pip install -e .

67-78: Remove duplicated Codecov token.

The Codecov token is specified both in the environment and in the action configuration. The token in the action configuration is sufficient.

  - name: Upload coverage
-   env:
-     CODECOV_TOKEN: ${{secrets.CODECOV_TOKEN}}
    uses: codecov/codecov-action@v3
    with:
      token: ${{ secrets.CODECOV_TOKEN }}
      directory: ./coverage/reports/
      fail_ci_if_error: true
      files: ./coverage.xml,!./cache
      flags: parsetests
      name: codecov-unittests
      verbose: true
test/parse_tests/test_parse_sv.py (3)

22-61: Well-structured test cases with good coverage.

The test cases effectively cover various Swedish date/time formats and scenarios. The helper method extractWithFormat makes the tests clean and maintainable.

Consider adding docstrings to document the expected behavior of each test method, similar to how you've done for test_extractdatetime_no_time.

 def test_extractdatetime_sv(self):
+    """Test extraction of dates and times from Swedish text with various formats."""
     def extractWithFormat(text):
         date = datetime(2017, 6, 27, 0, 0)

 def test_extractdatetime_default_sv(self):
+    """Test that default time is correctly applied when no specific time is provided."""
     default = time(9, 0, 0)
🧰 Tools
🪛 Ruff

25-25: Undefined name extract_datetime

(F821)


55-55: Undefined name extract_datetime

(F821)


61-61: Undefined name extract_datetime

(F821)


64-100: Comprehensive duration tests with good edge cases.

The test cases effectively cover various duration formats, including Swedish-specific terms like "kvart" and "halvtimme". Both valid and invalid cases are well tested.

Consider using a data-driven approach to make the tests more maintainable:

 def test_valid_extract_duration(self):
     """Duration in sentence."""
-    td, remains = extract_duration("5 minuter", lang='sv-se')
-    self.assertEqual(td, timedelta(seconds=300))
-    self.assertEqual(remains, '')
-    # ... more test cases
+    test_cases = [
+        ("5 minuter", timedelta(seconds=300), ''),
+        ("om 2 och en halv timme", timedelta(hours=2, minutes=30), "om och"),
+        # ... more cases
+    ]
+    for text, expected_td, expected_remains in test_cases:
+        td, remains = extract_duration(text, lang='sv-se')
+        self.assertEqual(td, expected_td)
+        self.assertEqual(remains, expected_remains)
🧰 Tools
🪛 Ruff

67-67: Undefined name extract_duration

(F821)


71-71: Undefined name extract_duration

(F821)


75-75: Undefined name extract_duration

(F821)


81-81: Undefined name extract_duration

(F821)


85-85: Undefined name extract_duration

(F821)


90-90: Undefined name extract_duration

(F821)


96-96: Undefined name extract_duration

(F821)


99-99: Undefined name extract_duration

(F821)


22-100: Consider adding more edge cases for robust testing.

While the current test coverage is good, consider adding these additional test cases:

  • Boundary times (23:59, 00:00)
  • Invalid date formats
  • Mixed language inputs (to ensure proper language detection)
  • Daylight saving time transitions

Would you like me to help generate additional test cases for these scenarios?

🧰 Tools
🪛 Ruff

25-25: Undefined name extract_datetime

(F821)


55-55: Undefined name extract_datetime

(F821)


61-61: Undefined name extract_datetime

(F821)


67-67: Undefined name extract_duration

(F821)


71-71: Undefined name extract_duration

(F821)


75-75: Undefined name extract_duration

(F821)


81-81: Undefined name extract_duration

(F821)


85-85: Undefined name extract_duration

(F821)


90-90: Undefined name extract_duration

(F821)


96-96: Undefined name extract_duration

(F821)


99-99: Undefined name extract_duration

(F821)

test/format_tests/test_format_pl.py (3)

28-84: Add docstring to explain test purpose and coverage.

While the test cases are comprehensive and well-structured, adding a docstring would help other developers understand:

  • The purpose of these tests
  • What aspects of Polish time formatting are being verified
  • Why certain test cases are important for Polish localization

Example docstring:

def test_convert_times(self):
    """Verify Polish time formatting for various scenarios.
    
    Tests both speech and 24-hour formats, covering:
    - Regular hours and minutes
    - Zero minutes handling
    - Midnight/Noon special cases
    - Hour/minute combinations specific to Polish grammar
    """

85-103: Consider implementing or removing commented test cases.

There are several commented-out test cases for non-speech duration formatting. These should either be:

  1. Implemented if the functionality exists
  2. Removed if the functionality isn't planned
  3. Converted to TODO comments if planned for future implementation

Would you like help implementing these test cases or creating a GitHub issue to track this work?


98-98: Consider breaking long line into multiple lines.

The line is marked with # nopep8 but could be made more readable by breaking it into multiple lines:

self.assertEqual(
    nice_duration(500000, "pl"),
    "pięć dni osiemnaście godzin pięćdziesiąt trzy minuty dwadzieścia sekund"
)
test/parse_tests/test_parse_fa.py (3)

28-55: Add documentation to improve test readability.

Consider adding:

  1. A docstring explaining the test method's purpose
  2. Comments translating Persian phrases for non-Persian speakers to aid maintenance

Example improvement:

 def test_extract_duration_fa(self):
+    """Test duration extraction from Persian (Farsi) text.
+    Verifies correct parsing of time units (seconds, minutes, hours, days, weeks)
+    and handling of decimal values and compound durations."""
     self.assertEqual(extract_duration("10 ثانیه"),  # "10 seconds"
                     (timedelta(seconds=10.0), ""))

57-62: Add docstring to helper function.

The extractWithFormat helper function would benefit from a docstring explaining its purpose, parameters, and return value.

Example improvement:

 def extractWithFormat(text):
+    """Helper function to extract and format datetime from Persian text.
+    
+    Args:
+        text (str): Persian text containing date/time information
+        
+    Returns:
+        list: [formatted_date_string, remaining_text]
+    """
     date = datetime(2017, 6, 27, 13, 4, tzinfo=default_timezone())

110-112: Clarify or resolve the TODO comment.

The TODO comment indicates a known issue with time extraction. Consider:

  1. Creating a GitHub issue to track this
  2. Adding more context about the specific problem
  3. Proposing a solution or acceptance criteria

Would you like me to help create a GitHub issue to track this improvement?

test/parse_tests/test_parse_nl.py (3)

30-41: Add docstrings to helper methods.

The helper methods extractWithFormat and testExtract would benefit from docstrings explaining their purpose, parameters, and return values.

Example implementation:

 def extractWithFormat(text):
+    """
+    Extract datetime from text and format it as string.
+    
+    Args:
+        text (str): Input text to parse
+    Returns:
+        list: [formatted_date_string, leftover_text]
+    """
     date = datetime(2017, 6, 27, 0, 0, tzinfo=default_timezone())
🧰 Tools
🪛 Ruff

33-33: Undefined name LANG

(F821)


126-127: Improve duration extraction cleanup.

The test case for complex duration ("een treinrit van 2 uur, 17 minuten en zestien seconden") shows messy leftover text: "1 treinrit van , en". This suggests the duration extraction could be improved to better handle surrounding text.

Consider enhancing the duration extractor to:

  1. Better handle numerical words (like "zestien")
  2. Clean up leftover punctuation and spaces
  3. Preserve the original text structure in leftover text
🧰 Tools
🪛 Ruff

126-126: Undefined name LANG

(F821)


107-109: Enhance test coverage with negative test cases.

While the test_extractdatetime_no_time method tests one negative case, consider adding more negative test cases to verify proper error handling for:

  • Invalid date formats
  • Out of range values
  • Malformed input

Example additions:

def test_extractdatetime_invalid_nl(self):
    """Test invalid date/time inputs."""
    self.assertEqual(extract_datetime("45e dag van de maand", lang=LANG), None)
    self.assertEqual(extract_datetime("13e maand", lang=LANG), None)
    self.assertEqual(extract_datetime("25:00 uur", lang=LANG), None)
🧰 Tools
🪛 Ruff

109-109: Undefined name LANG

(F821)

test/parse_tests/test_parse_da.py (3)

26-40: Consider making the anchor date configurable.

The hardcoded anchor date (2017-06-27) in extractWithFormat could make tests brittle. Consider making it a class-level constant or setUp parameter for better maintainability.

 class TestNormalize(unittest.TestCase):
+    ANCHOR_DATE = datetime(2017, 6, 27, 0, 0, tzinfo=default_timezone())
 
     def test_extractdatetime_da(self):
         def extractWithFormat(text):
-            date = datetime(2017, 6, 27, 0, 0, tzinfo=default_timezone())
+            date = self.ANCHOR_DATE
             [extractedDate, leftover] = extract_datetime(text, date,
                                                         lang="da-dk", )

41-108: Improve test case formatting consistency.

While the test coverage is comprehensive, there are formatting inconsistencies:

  • Inconsistent indentation in testExtract calls (e.g., lines 41-42 vs 52-53)
  • Some lines exceed standard line length

Consider using a consistent format:

testExtract(
    "sæt frisøraftale på fredag",
    "2017-06-30 00:00:00",
    "sæt frisøraftale"
)

116-126: Enhance default time test coverage.

While both test methods cover important scenarios, test_extractdatetime_default_da could be more thorough:

  • Only tests the time component
  • Could verify the complete datetime object
  • Could test multiple default time values

Consider expanding the test:

def test_extractdatetime_default_da(self):
    default = time(9, 0, 0)
    anchor = datetime(2017, 6, 27, 0, 0)
    res = extract_datetime(
        "lad os mødes på fredag klokken 9 om morgenen",
        anchor, lang='da-dk', default_time=default
    )
    expected = datetime(2017, 6, 30, 9, 0, 0)
    self.assertEqual(expected, res[0])
    self.assertEqual(default, res[0].time())
test/parse_tests/test_parse_eu.py (4)

22-71: Add docstring to describe test coverage.

Add a docstring to explain the test's purpose and the various month formats being tested.

 def test_datetime_by_date_eu(self):
+    """Test datetime extraction for various Basque month formats.
+    
+    Tests both abbreviated (urt, ots, etc.) and full month names (ekaina, uztaila)
+    with fixed dates and anchor dates.
+    """
     # test currentDate==None
🧰 Tools
🪛 Ruff

24-24: Undefined name now_local

(F821)


27-27: Undefined name extract_datetime_eu

(F821)


31-31: Undefined name extract_datetime

(F821)


33-33: Undefined name default_timezone

(F821)


34-34: Undefined name extract_datetime

(F821)


36-36: Undefined name default_timezone

(F821)


37-37: Undefined name extract_datetime

(F821)


39-39: Undefined name default_timezone

(F821)


40-40: Undefined name extract_datetime

(F821)


42-42: Undefined name default_timezone

(F821)


43-43: Undefined name extract_datetime

(F821)


45-45: Undefined name default_timezone

(F821)


46-46: Undefined name extract_datetime

(F821)


48-48: Undefined name default_timezone

(F821)


49-49: Undefined name extract_datetime

(F821)


51-51: Undefined name default_timezone

(F821)


52-52: Undefined name extract_datetime

(F821)


54-54: Undefined name default_timezone

(F821)


55-55: Undefined name extract_datetime

(F821)


57-57: Undefined name default_timezone

(F821)


58-58: Undefined name extract_datetime

(F821)


60-60: Undefined name default_timezone

(F821)


61-61: Undefined name extract_datetime

(F821)


63-63: Undefined name default_timezone

(F821)


64-64: Undefined name extract_datetime

(F821)


66-66: Undefined name default_timezone

(F821)


67-67: Undefined name extract_datetime

(F821)


69-69: Undefined name default_timezone

(F821)


71-71: Undefined name extract_datetime

(F821)


79-83: Rename test method to better reflect its purpose.

The current name test_bugged_output_wastebasket doesn't clearly indicate what's being tested. Consider a more descriptive name.

-def test_bugged_output_wastebasket(self):
+def test_datetime_with_year_parsing(self):
🧰 Tools
🪛 Ruff

81-81: Undefined name extract_datetime

(F821)


82-82: Undefined name default_timezone

(F821)


84-110: Add docstring to document relative date expressions.

Add a docstring to explain the various relative date expressions being tested.

 def test_extract_datetime_relative(self):
+    """Test datetime extraction for Basque relative expressions.
+    
+    Tests various relative time expressions including:
+    - Today/tonight (gaurko gaua)
+    - Yesterday (atzoko)
+    - Day before yesterday (herenegungo)
+    - Tomorrow morning (biharko goiza)
+    - Specific times (goizeko 2, arratsaldeko 2)
+    - Next year (datorren urtea)
+    """
🧰 Tools
🪛 Ruff

85-85: Undefined name extract_datetime

(F821)


86-86: Undefined name default_timezone

(F821)


87-87: Undefined name extract_datetime

(F821)


89-89: Undefined name default_timezone

(F821)


90-90: Undefined name extract_datetime

(F821)


91-91: Undefined name default_timezone

(F821)


92-92: Undefined name extract_datetime

(F821)


93-93: Undefined name default_timezone

(F821)


94-94: Undefined name extract_datetime

(F821)


95-95: Undefined name default_timezone

(F821)


96-96: Undefined name extract_datetime

(F821)


97-97: Undefined name default_timezone

(F821)


98-98: Undefined name extract_datetime

(F821)


99-99: Undefined name default_timezone

(F821)


100-100: Undefined name extract_datetime

(F821)


101-101: Undefined name default_timezone

(F821)


103-103: Undefined name extract_datetime

(F821)


104-104: Undefined name default_timezone

(F821)


105-105: Undefined name extract_datetime

(F821)


106-106: Undefined name default_timezone

(F821)


108-108: Undefined name extract_datetime

(F821)


109-109: Undefined name default_timezone

(F821)


20-120: Consider improving test organization and maintainability.

The test suite has good coverage but could benefit from some structural improvements:

  1. Consider grouping related tests using nested TestCase classes:
    • One for fixed dates
    • One for relative dates
    • One for edge cases
  2. Consider moving test data to a separate fixture file to improve maintainability
  3. Add type hints to improve code clarity

Example structure:

from typing import Optional
from datetime import datetime
from .fixtures.eu_test_data import TEST_DATES, RELATIVE_DATES

class TestBasqueDateParsing(unittest.TestCase):
    class TestFixedDates(unittest.TestCase):
        def test_datetime_by_date_eu(self) -> None: ...
        
    class TestRelativeDates(unittest.TestCase):
        def test_extract_datetime_relative(self) -> None: ...
        
    class TestEdgeCases(unittest.TestCase):
        def test_extractdatetime_no_time(self) -> None: ...
🧰 Tools
🪛 Ruff

24-24: Undefined name now_local

(F821)


27-27: Undefined name extract_datetime_eu

(F821)


31-31: Undefined name extract_datetime

(F821)


33-33: Undefined name default_timezone

(F821)


34-34: Undefined name extract_datetime

(F821)


36-36: Undefined name default_timezone

(F821)


37-37: Undefined name extract_datetime

(F821)


39-39: Undefined name default_timezone

(F821)


40-40: Undefined name extract_datetime

(F821)


42-42: Undefined name default_timezone

(F821)


43-43: Undefined name extract_datetime

(F821)


45-45: Undefined name default_timezone

(F821)


46-46: Undefined name extract_datetime

(F821)


48-48: Undefined name default_timezone

(F821)


49-49: Undefined name extract_datetime

(F821)


51-51: Undefined name default_timezone

(F821)


52-52: Undefined name extract_datetime

(F821)


54-54: Undefined name default_timezone

(F821)


55-55: Undefined name extract_datetime

(F821)


57-57: Undefined name default_timezone

(F821)


58-58: Undefined name extract_datetime

(F821)


60-60: Undefined name default_timezone

(F821)


61-61: Undefined name extract_datetime

(F821)


63-63: Undefined name default_timezone

(F821)


64-64: Undefined name extract_datetime

(F821)


66-66: Undefined name default_timezone

(F821)


67-67: Undefined name extract_datetime

(F821)


69-69: Undefined name default_timezone

(F821)


71-71: Undefined name extract_datetime

(F821)


81-81: Undefined name extract_datetime

(F821)


82-82: Undefined name default_timezone

(F821)


85-85: Undefined name extract_datetime

(F821)


86-86: Undefined name default_timezone

(F821)


87-87: Undefined name extract_datetime

(F821)


89-89: Undefined name default_timezone

(F821)


90-90: Undefined name extract_datetime

(F821)


91-91: Undefined name default_timezone

(F821)


92-92: Undefined name extract_datetime

(F821)


93-93: Undefined name default_timezone

(F821)


94-94: Undefined name extract_datetime

(F821)


95-95: Undefined name default_timezone

(F821)


96-96: Undefined name extract_datetime

(F821)


97-97: Undefined name default_timezone

(F821)


98-98: Undefined name extract_datetime

(F821)


99-99: Undefined name default_timezone

(F821)


100-100: Undefined name extract_datetime

(F821)


101-101: Undefined name default_timezone

(F821)


103-103: Undefined name extract_datetime

(F821)


104-104: Undefined name default_timezone

(F821)


105-105: Undefined name extract_datetime

(F821)


106-106: Undefined name default_timezone

(F821)


108-108: Undefined name extract_datetime

(F821)


109-109: Undefined name default_timezone

(F821)


113-113: Undefined name extract_datetime

(F821)


117-117: Undefined name extract_datetime

(F821)


118-118: Undefined name default_timezone

(F821)

test/format_tests/test_format_it.py (3)

27-28: Add class docstring to describe test purpose.

Add a docstring to describe the purpose of this test class and what aspects of Italian time formatting it verifies.

 class Testtimes(unittest.TestCase):
+    """Test suite for Italian time formatting using nice_time function.
+    
+    This class verifies the correct formatting of times in Italian, including:
+    - Standard time formatting
+    - AM/PM variations
+    - 24-hour format
+    - Special cases (midnight, noon)
+    - Speech vs non-speech output
+    """

51-66: Group related test cases using subtests.

The time period tests (morning, evening, night) would be clearer using unittest's subTest context manager. This helps identify which specific time period failed in test reports.

def test_time_periods(self):
    """Test time period specific formatting (morning, evening, night)."""
    test_cases = [
        (8, 22, "della mattina"),
        (20, 22, "della sera"),
        (23, 22, "della notte"),
        (0, 0, "mezzanotte"),
        (12, 0, "mezzogiorno")
    ]
    for hour, minute, expected_period in test_cases:
        with self.subTest(hour=hour, minute=minute):
            dt = datetime.datetime(2017, 1, 31, hour, minute, 3,
                                 tzinfo=default_timezone())
            result = nice_time(dt, lang="it", use_ampm=True)
            self.assertIn(expected_period, result)

121-133: Improve test case documentation for edge cases.

The "casi particolari" (special cases) section would benefit from more detailed comments explaining what each case is testing and why it's special.

-        # casi particolari
+        # Special cases testing specific edge conditions:
+        # 1. 1:02 - Single digit hour in 24h format
+        # 2. 2:02 - Leading zero in speech format
+        # 3. 10:15 - Quarter hour formatting
+        # 4. 22:45 - Three quarters hour formatting
test/parse_tests/test_parse_ca.py (4)

31-38: Consider parameterizing the anchor date.

The anchor date is hardcoded to 2017, which might make the tests less maintainable. Consider making it a class-level constant or using a more recent date.

-            date = datetime(2017, 6, 27, 0, 0, tzinfo=default_timezone())
+            ANCHOR_DATE = datetime(2024, 1, 1, 0, 0, tzinfo=default_timezone())
+            date = ANCHOR_DATE

135-136: Address or remove commented test case.

There's a commented test case that should either be fixed and uncommented or removed if no longer needed.


44-141: Consider organizing test cases into logical groups.

The test cases cover various scenarios but could be better organized using subtest contexts. Consider grouping related test cases (e.g., relative dates, specific times, future dates) using unittest.TestCase.subTest().

Example refactor:

def test_extractdatetime_ca(self):
    # ... helper functions ...
    
    with self.subTest("Basic date tests"):
        testExtract("quin dia és avui", "2017-06-27 00:00:00", "dia")
        testExtract("quin dia som avui", "2017-06-27 00:00:00", "dia")
    
    with self.subTest("Relative dates"):
        testExtract("quin dia és demà", "2017-06-28 00:00:00", "dia")
        testExtract("quin dia va ser ahir", "2017-06-26 00:00:00", "dia ser")
    
    # ... more groups ...

142-149: Consider adding more default time test cases.

The default time testing is minimal. Consider adding more test cases to cover:

  • Different default times (morning, afternoon, evening)
  • Edge cases (midnight, noon)
  • Invalid default times
test/format_tests/test_format.py (3)

77-79: Address the TODO comment about language loading.

The TODO comment indicates potential issues with language loading. This should be addressed to ensure reliable testing across all languages.

Would you like me to help create a GitHub issue to track this language loading enhancement?


49-75: Refactor test_nice_date for better maintainability.

The test method contains nested loops and multiple print statements. Consider refactoring for better readability and maintainability.

 def test_nice_date(self):
+    def _test_specific_dates(self, lang):
+        i = 1
+        while (self.test_config[lang].get('test_nice_date') and
+               self.test_config[lang]['test_nice_date'].get(str(i))):
+            test_case = self.test_config[lang]['test_nice_date'][str(i)]
+            dp = ast.literal_eval(test_case['datetime_param'])
+            np = ast.literal_eval(test_case['now'])
+            dt = datetime.datetime(*dp)
+            now = None if not np else datetime.datetime(*np)
+            logging.info('Testing for %s that %s is date %s',
+                        lang, dt, test_case['assertEqual'])
+            self.assertEqual(test_case['assertEqual'],
+                           nice_date(dt, lang=lang, now=now))
+            i += 1
+
+    def _test_full_year(self, lang):
+        start_date = datetime.datetime(2017, 12, 30, 0, 2, 3)
+        for days in range(368):
+            dt = start_date + datetime.timedelta(days=days)
+            self.assertTrue(len(nice_date(dt, lang=lang)) > 0)
+
     for lang in self.test_config:
-        i = 1
-        while (self.test_config[lang].get('test_nice_date') and
-               self.test_config[lang]['test_nice_date'].get(str(i))):
-            p = self.test_config[lang]['test_nice_date'][str(i)]
-            # ... rest of the code
+        _test_specific_dates(self, lang)
+        _test_full_year(self, lang)

130-190: Consider using parameterized tests for better maintainability.

The test methods could benefit from parameterization to make them more maintainable and easier to extend with new test cases.

+from parameterized import parameterized
+
 class TestNiceDateUtils(unittest.TestCase):
     @classmethod
     def setUpClass(cls):
         cls.lang = 'en-us'
 
-    def test_nice_day(self):
-        # Test with include_month=True
-        dt = datetime.datetime(2022, 10, 31)
-        self.assertEqual(nice_day(dt, self.lang, 'MDY', True), "October 31")
-        self.assertEqual(nice_day(dt, self.lang, 'DMY', True), "31 October")
+    @parameterized.expand([
+        ("MDY_with_month", True, "MDY", True, "October 31"),
+        ("DMY_with_month", True, "DMY", True, "31 October"),
+        ("without_month", False, "MDY", False, "31"),
+    ])
+    def test_nice_day(self, name, include_month, date_format, expected):
+        dt = datetime.datetime(2022, 10, 31)
+        self.assertEqual(
+            nice_day(dt, self.lang, date_format, include_month),
+            expected
+        )
test/format_tests/test_format_en.py (2)

27-29: Add docstring to test class.

Add a class-level docstring describing the purpose and scope of these tests.

 class TestNiceDateFormat(unittest.TestCase):
+    """Test suite for English date/time formatting functions.
+    
+    Tests various combinations of time formats including:
+    - 12/24 hour time
+    - AM/PM indicators
+    - Special time phrases (quarter past, half past, etc.)
+    """

29-148: Consider using parameterized tests to reduce code duplication.

While the test coverage is comprehensive, there's significant repetition in the test structure. Consider using @pytest.mark.parametrize or unittest.TestCase.subTest to make the tests more maintainable.

Example refactor for the first test case:

def test_convert_times(self):
    test_cases = [
        {
            'dt': datetime.datetime(2017, 1, 31, 13, 22, 3, tzinfo=default_timezone()),
            'expected': {
                'basic': "one twenty two",
                'ampm': "one twenty two p.m.",
                'no_speech': "1:22",
                'no_speech_ampm': "1:22 PM",
                '24h': "13:22",
                '24h_speech': "thirteen twenty two"
            }
        },
        # ... more test cases
    ]
    
    for case in test_cases:
        with self.subTest(dt=case['dt']):
            self.assertEqual(nice_time(case['dt'], "en"), case['expected']['basic'])
            self.assertEqual(nice_time(case['dt'], "en", use_ampm=True), case['expected']['ampm'])
            # ... more assertions
test/format_tests/test_format_fa.py (3)

27-39: Enhance error handling in test setup.

Consider these improvements for better robustness:

  1. Add error handling for JSON parsing
  2. Replace print with proper logging
  3. Validate loaded test data structure
 @classmethod
 def setUpClass(cls):
     # Read date_time_test.json files for test data
     cls.test_config = {}
     p = Path(date_time_format.config_path)
     for sub_dir in [x for x in p.iterdir() if x.is_dir()]:
         if (sub_dir / 'date_time_test.json').exists():
-            print("Getting test for " +
-                  str(sub_dir / 'date_time_test.json'))
+            import logging
+            logging.debug("Loading test config from %s", 
+                         sub_dir / 'date_time_test.json')
             with (sub_dir / 'date_time_test.json').open() as f:
-                cls.test_config[sub_dir.parts[-1]] = json.loads(f.read())
+                try:
+                    data = json.loads(f.read())
+                    # Basic validation of expected structure
+                    if isinstance(data, dict):
+                        cls.test_config[sub_dir.parts[-1]] = data
+                    else:
+                        logging.error("Invalid test config format in %s", 
+                                    sub_dir)
+                except json.JSONDecodeError as e:
+                    logging.error("Failed to parse test config %s: %s", 
+                                sub_dir, e)

40-158: Consider improving test structure and documentation.

The test coverage is comprehensive, but could benefit from:

  1. Using parameterized tests to reduce code duplication
  2. Adding constants for test dates/times
  3. Including docstrings to explain test scenarios

Example refactor using parameterized library:

from parameterized import parameterized

class TestNiceDateFormat(unittest.TestCase):
    # Constants for commonly used test times
    TEST_TIME_1 = datetime.datetime(2017, 1, 31, 13, 22, 3)
    TEST_TIME_2 = datetime.datetime(2017, 1, 31, 13, 0, 3)
    
    @parameterized.expand([
        ("basic", TEST_TIME_1, {}, "یک و بیست و دو دقیقه"),
        ("with_ampm", TEST_TIME_1, {"use_ampm": True}, 
         "یک و بیست و دو دقیقه بعد از ظهر"),
        ("non_speech", TEST_TIME_1, {"speech": False}, "1:22"),
        # ... more test cases
    ])
    def test_nice_time_formats(self, name, dt, kwargs, expected):
        """Test nice_time function with various formatting options.
        
        Args:
            name: Test case name
            dt: Input datetime
            kwargs: Formatting options
            expected: Expected output string
        """
        self.assertEqual(nice_time(dt, "fa-ir", **kwargs), expected)

159-178: Track the Persian duration formatting issue.

The commented tests indicate a known issue with Persian duration formatting due to the "و" (and) conjunction. This should be properly tracked and addressed.

Would you like me to:

  1. Create a GitHub issue to track this bug?
  2. Help implement a fix for the Persian duration formatting?
test/format_tests/test_format_da.py (2)

25-26: Add docstrings to improve test documentation.

Please add docstrings to both the test class and method to describe:

  • The purpose of the test suite
  • The different scenarios being tested
  • The expected behavior for each format configuration

Example:

class TestNiceDateFormat_da(unittest.TestCase):
    """Test suite for Danish time formatting using nice_time function."""

    def test_convert_times_da(self):
        """Test nice_time formatting for Danish language.
        
        Tests multiple scenarios including:
        - 12/24 hour format
        - With/without AM/PM
        - Speech/non-speech output
        - Various time combinations
        """

26-181: Consider splitting the test method into smaller, focused test cases.

The current test method is quite long and tests multiple scenarios. Consider splitting it into smaller, focused test methods for better maintainability and clarity:

def test_afternoon_time_speech(self):
    """Test speech output for afternoon times."""
    dt = datetime.datetime(2017, 1, 31, 13, 22, 3, tzinfo=default_timezone())
    self.assertEqual(nice_time(dt, lang="da-dk"), "et toogtyve")
    self.assertEqual(nice_time(dt, lang="da-dk", use_ampm=True),
                     "et toogtyve om eftermiddagen")

def test_afternoon_time_numeric(self):
    """Test numeric output for afternoon times."""
    dt = datetime.datetime(2017, 1, 31, 13, 22, 3, tzinfo=default_timezone())
    self.assertEqual(nice_time(dt, lang="da-dk", speech=False), "01:22")
    self.assertEqual(nice_time(dt, lang="da-dk", speech=False, use_ampm=True),
                     "01:22 PM")

# Additional methods for other scenarios...

This would:

  • Improve test readability
  • Make it easier to identify which scenario failed
  • Allow for better test maintenance
  • Enable running specific test scenarios
test/format_tests/test_format_sv.py (1)

26-27: Add class docstring to document test purpose.

Add a docstring explaining that this class tests Swedish time formatting.

 class TestNiceDateFormat_sv(unittest.TestCase):
+    """Test suite for Swedish (sv-se) time formatting using nice_time function."""
test/format_tests/test_format_eu.py (3)

27-29: Fix datetime constructor indentation.

The indentation in datetime constructor calls is inconsistent with Python style guidelines.

-dt = datetime.datetime(2017, 1, 31,
-                               13, 22, 3)
+dt = datetime.datetime(2017, 1, 31,
+                      13, 22, 3)

Also applies to: 50-52, 66-68, 85-87, 107-109, 131-133, 155-157, 160-162, 165-167, 170-172, 175-177, 180-182, 185-187


31-35: Standardize language code usage.

The code inconsistently uses both "eu" and "eu-eu" language codes. Choose one standard format throughout the tests.

-self.assertEqual(nice_time(dt, lang="eu-eu"),
+self.assertEqual(nice_time(dt, lang="eu"),

Also applies to: 109-111


30-83: Add descriptive comments for test scenarios.

Add comments to describe the specific scenarios being tested in each block. This helps other developers understand the test coverage.

Example structure:

# Test default formatting
self.assertEqual(...)

# Test AM/PM formatting
self.assertEqual(...)

# Test 24-hour formatting
self.assertEqual(...)
test/parse_tests/test_parse_es.py (3)

16-23: Consider reorganizing imports for better readability

Consider reorganizing imports into standard groups:

  1. Standard library imports
  2. Third-party imports
  3. Local imports
 import unittest
 from datetime import datetime, timedelta
-
 from ovos_config.locale import get_default_tz as default_timezone
-
 from ovos_date_parser import (
     extract_duration, extract_datetime
 )

66-93: Well documented skipped tests, consider adding issue tracking

Good practice keeping the failing tests with @unittest.skip and documenting the issues bilingually. However, consider:

  1. Adding a link to your issue tracker for these specific test failures
  2. Adding expected completion timeline for fixing these issues

Would you like me to help create GitHub issues to track these failing test cases?

Also applies to: 125-142


26-189: Consider adding more edge cases to improve test coverage

The test suite has good coverage of basic functionality, but consider adding tests for:

  1. Invalid inputs (malformed dates, out of range values)
  2. Mixed language inputs
  3. Timezone edge cases
  4. Leap year dates
  5. Date format variations (formal vs colloquial)

Would you like me to help generate additional test cases for these scenarios?

🧰 Tools
🪛 Ruff

33-33: Undefined name extract_datetime_es

(F821)


166-166: Undefined name DAYS_IN_1_MONTH

(F821)


169-169: Undefined name DAYS_IN_1_MONTH

(F821)


171-171: Undefined name DAYS_IN_1_YEAR

(F821)


173-173: Undefined name DAYS_IN_1_YEAR

(F821)


175-175: Undefined name DAYS_IN_1_YEAR

(F821)


177-177: Undefined name DAYS_IN_1_YEAR

(F821)


179-179: Undefined name DAYS_IN_1_YEAR

(F821)


181-181: Undefined name DAYS_IN_1_YEAR

(F821)


183-183: Undefined name DAYS_IN_1_YEAR

(F821)


185-185: Undefined name DAYS_IN_1_YEAR

(F821)

test/format_tests/test_format_nl.py (1)

26-26: Add class docstring to explain test purpose.

Add a docstring to explain that this class tests Dutch language time formatting.

 class TestNiceDateFormat_nl(unittest.TestCase):
+    """Test suite for Dutch language time formatting using nice_time function."""
test/format_tests/test_format_hu.py (2)

26-27: Add class docstring to explain test purpose.

Add a docstring to explain the purpose of these Hungarian time format tests and any specific cultural considerations.

 class TestNiceDateFormat_hu(unittest.TestCase):
+    """Test the nice_time function for Hungarian language formatting.
+    
+    These tests verify various time formats including:
+    - 12/24 hour time
+    - With/without AM/PM
+    - Speech/numeric output
+    - Special time periods (morning, afternoon, evening, night)
+    """

28-29: Reduce code duplication in datetime construction.

Consider creating a helper method to construct datetime objects, reducing repetition and improving maintainability.

def _create_datetime(self, hour, minute, second=0):
    """Create a datetime object with default test date and timezone."""
    return datetime.datetime(2017, 1, 31, hour, minute, second, 
                           tzinfo=default_timezone())

Then use it in tests:

dt = self._create_datetime(13, 22, 3)

Also applies to: 53-54, 77-78, 101-102, 125-126, 149-150, 173-174, 178-179, 183-184, 188-189, 193-194, 198-199

test/format_tests/test_format_fr.py (2)

26-27: Consider improving test structure and documentation.

While the test implementation is thorough, consider these improvements:

  1. Split test_convert_times_fr into smaller, focused test methods (e.g., test_24hour_format, test_special_times, test_quarter_hours)
  2. Add docstrings to describe the test class and methods
  3. Consider using parameterized tests to reduce code duplication

Example structure:

class TestNiceDateFormat_fr(unittest.TestCase):
    """Test suite for French time formatting."""
    
    def test_24hour_format(self):
        """Test 24-hour time formatting."""
        
    def test_special_times(self):
        """Test midi/minuit special cases."""
        
    def test_quarter_hours(self):
        """Test quarter and half hour formatting."""

27-202: Consider adding tests for edge cases.

The test coverage is comprehensive but could be enhanced by adding:

  1. Tests for 59 minutes (e.g., "une heure moins une minute")
  2. Tests for invalid inputs (None, invalid datetime objects)
  3. Tests for different date components to ensure they don't affect time formatting
test/format_tests/test_format_es.py (2)

27-28: Add docstring to describe test class purpose.

The test class would benefit from a docstring explaining its purpose and the functionality being tested.

 class TestNiceDateFormat(unittest.TestCase):
+    """Test the nice_time function's Spanish language (es-es) formatting capabilities."""

1-198: Consider implementing a base test class for common time formatting scenarios.

Since this test file is part of a larger suite testing the same functionality across different languages, consider creating a base test class that implements common test scenarios. Language-specific test classes could inherit from this base class and override or extend test cases as needed.

Benefits:

  • Reduces code duplication across language test files
  • Ensures consistent test coverage across languages
  • Makes it easier to add new test cases for all languages
  • Simplifies maintenance

Example structure:

class BaseNiceDateFormatTest(unittest.TestCase):
    """Base class for testing nice_time function across languages."""
    
    def setUp(self):
        self.lang = None  # Override in subclasses
    
    def test_basic_time_formatting(self):
        # Common test logic with language-specific expected results
        pass

class SpanishNiceDateFormatTest(BaseNiceDateFormatTest):
    """Test nice_time function for Spanish language."""
    
    def setUp(self):
        self.lang = "es-es"
test/format_tests/test_format_pt.py (3)

29-30: Fix inconsistent indentation in datetime constructor calls.

The datetime constructor calls have inconsistent indentation. The parameters should be aligned with the opening parenthesis.

Apply this formatting:

-        dt = datetime.datetime(2017, 1, 31,
-                               13, 22, 3, tzinfo=default_timezone())
+        dt = datetime.datetime(2017, 1, 31,
+                             13, 22, 3, tzinfo=default_timezone())

Also applies to: 52-53, 88-89, 111-112, 137-138, 163-164, 170-171, 175-176, 180-181, 185-186, 190-191, 195-196


28-28: Add docstrings to test methods.

Each test method should have a docstring describing the test scenario and expected behavior. This improves test maintainability and helps other developers understand the test cases.

Example for the first test method:

def test_pm(self):
    """Test afternoon time formatting.
    
    Verifies that PM times are correctly formatted in both 12 and 24-hour formats,
    with and without AM/PM indicators, in both speech and non-speech modes.
    """

Also applies to: 87-87, 110-110, 135-135, 161-161


28-203: Consider refactoring test methods for better organization.

The test methods are comprehensive but quite long. Consider:

  1. Creating helper methods for common assertion patterns
  2. Using parameterized tests to reduce code duplication
  3. Breaking down large test methods into smaller, focused ones

This would improve maintainability and make the test cases easier to understand.

Example helper method:

def assert_time_formats(self, dt, expected_speech, expected_speech_ampm,
                       expected_numeric, expected_numeric_ampm,
                       expected_24h, expected_24h_ampm):
    """Helper method to assert all time format variations."""
    self.assertEqual(nice_time(dt, lang="pt"), expected_speech)
    self.assertEqual(nice_time(dt, lang="pt", use_ampm=True),
                    expected_speech_ampm)
    self.assertEqual(nice_time(dt, lang="pt", speech=False),
                    expected_numeric)
    self.assertEqual(nice_time(dt, lang="pt", speech=False,
                    use_ampm=True), expected_numeric_ampm)
    self.assertEqual(nice_time(dt, lang="pt", speech=False,
                    use_24hour=True), expected_24h)
    self.assertEqual(nice_time(dt, lang="pt", speech=False,
                    use_24hour=True, use_ampm=True), expected_24h_ampm)
test/parse_tests/test_parse_pt.py (3)

29-95: Add docstrings to helper methods and consider splitting test cases

The test cases are comprehensive, but the code could be more maintainable with:

  1. Docstrings for extractWithFormat and testExtract methods explaining their purpose and parameters
  2. Splitting the large test method into smaller, focused test methods grouped by type (e.g., relative dates, specific times, combined expressions)

Example docstring for extractWithFormat:

def extractWithFormat(text):
    """
    Helper method to extract datetime from Portuguese text with standard formatting.
    
    Args:
        text (str): Portuguese text containing date/time expression
        
    Returns:
        list: [formatted_date_string, leftover_text]
    """
🧰 Tools
🪛 Ruff

31-31: Undefined name default_timezone

(F821)


152-158: Address TODO comments about imperfect remainders

The TODOs indicate known issues with remainder handling in complex duration expressions. These should be tracked and addressed.

Would you like me to create GitHub issues to track these TODOs and propose solutions?


136-195: Consider adding more edge cases

The test suite is comprehensive but could benefit from additional test cases:

  1. Invalid/malformed input handling
  2. Mixed numeric and text representations (e.g., "2 anos e três meses")
  3. Boundary values for each time unit
  4. Negative durations if supported

Would you like me to provide examples of these additional test cases?

🧰 Tools
🪛 Ruff

166-166: Undefined name DAYS_IN_1_MONTH

(F821)


169-169: Undefined name DAYS_IN_1_MONTH

(F821)


172-172: Undefined name DAYS_IN_1_MONTH

(F821)


174-174: Undefined name DAYS_IN_1_YEAR

(F821)


176-176: Undefined name DAYS_IN_1_YEAR

(F821)


178-178: Undefined name DAYS_IN_1_YEAR

(F821)


180-180: Undefined name DAYS_IN_1_YEAR

(F821)


182-182: Undefined name DAYS_IN_1_YEAR

(F821)


184-184: Undefined name DAYS_IN_1_YEAR

(F821)


186-186: Undefined name DAYS_IN_1_YEAR

(F821)


188-188: Undefined name DAYS_IN_1_YEAR

(F821)


190-190: Undefined name DAYS_IN_1_YEAR

(F821)


192-192: Undefined name DAYS_IN_1_YEAR

(F821)


194-194: Undefined name DAYS_IN_1_YEAR

(F821)

test/format_tests/test_format_sl.py (3)

32-39: Add error handling for config file loading.

The setup could be more robust with proper error handling and path construction:

  1. Missing error handling for file not found scenario
  2. Hardcoded language code could be moved to a class variable

Consider this improvement:

 @classmethod
 def setUpClass(cls):
+    cls.language = "sl"
     # Read date_time_test.json files for test data
-    language = "sl"
-    config = date_time_format.config_path + "/" + language + "/date_time_test.json"
+    config = os.path.join(date_time_format.config_path, cls.language, "date_time_test.json")
 
     cls.test_config = {}
-    with open(config, encoding="utf8") as file:
-        cls.test_config[language] = json.loads(file.read())
+    try:
+        with open(config, encoding="utf8") as file:
+            cls.test_config[cls.language] = json.loads(file.read())
+    except FileNotFoundError:
+        raise RuntimeError(f"Test config file not found for language: {cls.language}")
+    except json.JSONDecodeError:
+        raise RuntimeError(f"Invalid JSON in test config file for language: {cls.language}")

41-183: Improve test organization and readability.

The test_convert_times method is quite long and could benefit from:

  1. Breaking down into smaller, focused test methods
  2. Using helper methods for datetime creation
  3. Adding descriptive docstrings for test scenarios

Consider this structure:

def _create_test_datetime(self, hour, minute, second=0):
    """Helper method to create test datetime objects."""
    return datetime.datetime(2017, 1, 31, hour, minute, second, tzinfo=default_timezone())

def test_basic_time_format(self):
    """Test basic time formatting without AM/PM."""
    dt = self._create_test_datetime(13, 22)
    self.assertEqual(nice_time(dt, "sl"), "dvaindvajset čez ena")
    
def test_ampm_time_format(self):
    """Test time formatting with AM/PM."""
    dt = self._create_test_datetime(13, 22)
    self.assertEqual(nice_time(dt, "sl", use_ampm=True), "dvaindvajset čez ena p.m.")

# Continue with similar focused test methods...

184-211: Address skipped duration tests and plural support.

The duration tests are currently skipped due to missing plural support implementation. The commented results indicate the expected correct behavior for Slovenian plural forms.

Would you like me to:

  1. Help implement the plural support for Slovenian language?
  2. Create a GitHub issue to track this enhancement?
  3. Generate a test implementation that handles the plural cases correctly?

Note: Slovenian has specific plural rules for numbers 1-4, which need to be handled differently from numbers 5 and above.

test/format_tests/test_format_cs.py (5)

25-32: Remove unused import nice_duration

The nice_duration function is imported but never used in this test file.

 from ovos_date_parser import (
     date_time_format,
     nice_date,
     nice_date_time,
     nice_time,
-    nice_duration,
     nice_year
 )
🧰 Tools
🪛 Ruff

30-30: ovos_date_parser.nice_duration imported but unused

Remove unused import: ovos_date_parser.nice_duration

(F401)


43-44: Replace print statement with proper logging

Debug print statements should be replaced with proper logging for better control over output verbosity.

-                print("Načítám test pro " +
-                      str(sub_dir / 'date_time_test.json'))
+                import logging
+                logging.debug("Loading test data from %s",
+                            str(sub_dir / 'date_time_test.json'))

187-191: Remove or implement commented test code

The commented out test for English fallback should either be implemented or removed. If this is a known limitation, document it in a TODO comment with a ticket reference.


181-182: Replace print statements with proper test frameworks

Debug print statements in test methods should use proper test frameworks for better output control. Consider using unittest's built-in verbosity levels or a proper test runner.

-            print('Testing for ' + lang + ' that ' + str(dt) +
-                  ' is date ' + p['assertEqual'])
+            self.longMessage = True
+            msg = f'Testing for {lang} that {dt} is date {p["assertEqual"]}'
+            # The message will only be displayed if the test fails

Also applies to: 214-215, 234-235, 242-242


167-247: Add docstrings to test methods

Test methods lack docstrings describing their purpose and test strategy. This documentation is crucial for maintaining and understanding the test suite.

Example for one method:

     def test_nice_date(self):
+        """Test nice_date function with Czech language.
+        
+        Tests various date formatting scenarios using test data from JSON:
+        - Current date handling
+        - Future dates
+        - Past dates
+        - Edge cases
+        
+        Test data is loaded from date_time_test.json in the Czech locale directory.
+        """
         lang = "cs"
test/format_tests/test_format_de.py (2)

29-204: Consider breaking down the test method for better maintainability.

While the test coverage is comprehensive, the test_convert_times_de method is quite long (175 lines). Consider breaking it down into smaller, focused test methods for better organization and maintainability:

-def test_convert_times_de(self):
+def test_standard_time_afternoon(self):
     dt = datetime.datetime(2017, 1, 31, 13, 22, 3, tzinfo=default_timezone())
     # Test cases for 13:22
     
+def test_exact_hour(self):
     dt = datetime.datetime(2017, 1, 31, 13, 0, 3, tzinfo=default_timezone())
     # Test cases for 13:00
     
+def test_midnight_hour(self):
     dt = datetime.datetime(2017, 1, 31, 0, 2, 3, tzinfo=default_timezone())
     # Test cases for 00:02
     
+def test_special_expressions(self):
     # Test cases for "viertel eins", "halb sechs", etc.

This organization would:

  • Make it easier to identify which specific time format is failing
  • Improve test maintenance and readability
  • Allow for better test failure isolation

206-263: Enhance test coverage with additional scenarios.

While the basic test coverage is good, consider adding these scenarios to make it more robust:

  1. Edge cases for nice_day:

    • First/last day of month
    • Leap year dates
  2. Error scenarios for get_date_strings:

    • Invalid date formats
    • None/invalid input handling
  3. German-specific cases:

    • Special date expressions
    • Holiday dates

Example additional test cases:

def test_nice_day_edge_cases(self):
    # First day of month
    dt = datetime.datetime(2022, 10, 1)
    self.assertEqual(nice_day(dt, "de", 'DMY', True), "1 Oktober")
    
    # Last day of month
    dt = datetime.datetime(2022, 10, 31)
    self.assertEqual(nice_day(dt, "de", 'DMY', True), "31 Oktober")
    
    # Leap year
    dt = datetime.datetime(2024, 2, 29)
    self.assertEqual(nice_day(dt, "de", 'DMY', True), "29 Februar")

def test_get_date_strings_error_cases(self):
    with self.assertRaises(ValueError):
        get_date_strings(None, lang="de")
    with self.assertRaises(ValueError):
        get_date_strings(datetime.datetime.now(), date_format='invalid', lang="de")
test/parse_tests/test_parse_de.py (5)

26-38: Consider making helper methods class methods.

The extractWithFormat and testExtract helper methods could be class methods as they don't use instance state.

-    def extractWithFormat(text):
+    @classmethod
+    def extractWithFormat(cls, text):

-    def testExtract(text, expected_date, expected_leftover):
+    @classmethod
+    def testExtract(cls, text, expected_date, expected_leftover):

127-133: Document TTS failure cases.

There are comments indicating "TTS failure" without explanation. Please document:

  1. What TTS issues are being tested
  2. Why these cases are important
  3. How the parser handles these edge cases

26-204: Consider using parameterized tests for better maintainability.

The test cases could be more maintainable using parameterized tests. This would:

  1. Make it easier to add new test cases
  2. Improve readability
  3. Reduce code duplication

Example refactor:

@pytest.mark.parametrize("text,expected_date,expected_leftover", [
    ("mache den frisörtermin in einer halben stunde",
     "2017-06-27 00:30:00", "mache frisörtermin"),
    ("mache den frisörtermin in drei stunden",
     "2017-06-27 03:00:00", "mache frisörtermin"),
    # ... more test cases
])
def test_extractdatetime_de(self, text, expected_date, expected_leftover):
    res = self.extractWithFormat(text)
    assert res[0] == expected_date
    assert res[1] == expected_leftover

236-238: Ensure language consistency in test cases.

This test case uses English input ("seven") while testing German parsing. Consider using the German equivalent "sieben" consistently.


270-272: Add English translation for German comments.

Please add English translation for the German comment to maintain documentation accessibility.

-        # wenn überhaupt wäre anstatt -sekunde -sekündig[e][ns] notwendig
+        # wenn überhaupt wäre anstatt -sekunde -sekündig[e][ns] notwendig
+        # (if at all, -sekündig[e][ns] would be necessary instead of -sekunde)
test/format_tests/test_format_az.py (7)

13-14: Fix typo in license header

There's a typo in the license header where "və" (Azerbaijani) is used instead of "and" (English). License headers should be in English.

-# See the License for the specific language governing permissions və
+# See the License for the specific language governing permissions and

37-46: Add error handling for missing test configuration files

The setup could be more robust by handling cases where the JSON file is missing or malformed.

 @classmethod
 def setUpClass(cls):
     cls.test_config = {}
     p = Path(date_time_format.config_path)
     for sub_dir in [x for x in p.iterdir() if x.is_dir()]:
         if (sub_dir / 'date_time_test.json').exists():
             print("Getting test for " + str(sub_dir / 'date_time_test.json'))
-            with (sub_dir / 'date_time_test.json').open() as f:
-                cls.test_config[sub_dir.parts[-1]] = json.loads(f.read())
+            try:
+                with (sub_dir / 'date_time_test.json').open() as f:
+                    cls.test_config[sub_dir.parts[-1]] = json.loads(f.read())
+            except json.JSONDecodeError as e:
+                print(f"Error loading {sub_dir}: {e}")
+                cls.test_config[sub_dir.parts[-1]] = {}

48-167: Consider using parameterized tests for better maintainability

The time formatting tests contain multiple similar test cases with different inputs. Consider using @pytest.mark.parametrize or unittest.TestCase.subTest to make the tests more maintainable and reduce code duplication.

Example refactor using subTest:

def test_convert_times(self):
    test_cases = [
        {
            'dt': datetime.datetime(2017, 1, 31, 13, 22, 3, tzinfo=default_timezone()),
            'expected': {
                'default': "ikiyə iyirmi iki dəqiqə işləyib",
                'ampm': "gündüz ikiyə iyirmi iki dəqiqə işləyib",
                'speech_false': "1:22",
                # ... add other variations
            }
        },
        # ... add other test cases
    ]
    
    for case in test_cases:
        with self.subTest(dt=case['dt']):
            self.assertEqual(nice_time(case['dt'], "az-az"),
                           case['expected']['default'])
            self.assertEqual(nice_time(case['dt'], "az-az", use_ampm=True),
                           case['expected']['ampm'])
            # ... add other assertions

168-189: Add docstring to explain test data structure

The test relies on external JSON data, but its structure is not documented. Add a docstring explaining the expected format of the test data.

 def test_nice_date(self):
+    """
+    Test nice_date function using test cases from JSON config.
+    
+    Expected JSON structure:
+    {
+        "test_nice_date": {
+            "1": {
+                "datetime_param": "[year, month, day, hour, minute, second]",
+                "now": "[year, month, day, hour, minute, second]",
+                "assertEqual": "expected output string"
+            }
+        }
+    }
+    """

191-193: Track TODO comment in issue tracker

The TODO comment about language loading features should be tracked in the issue tracker for better visibility and follow-up.

Would you like me to create a GitHub issue to track this technical debt?


233-238: Optimize year range test

Testing all years from 1 to 9999 might be excessive and time-consuming. Consider testing a representative sample or key boundary values instead.

-        for i in range(1, 9999):
+        # Test boundary values and representative samples
+        test_years = [1, 2, 100, 1000, 1999, 2000, 2024, 9998, 9999]
+        for i in test_years:
             dt = datetime.datetime(i, 1, 31, 13, 2, 3, tzinfo=default_timezone())
             self.assertTrue(len(nice_year(dt, lang=lang)) > 0)

240-258: Split duration tests by format type

Consider splitting the duration tests into separate methods for speech and numerical formats to improve test organization and readability.

def test_nice_duration_speech(self):
    """Test duration formatting in speech format."""
    self.assertEqual(nice_duration(1, "az-az"), "bir saniyə")
    self.assertEqual(nice_duration(3, "az-az"), "üç saniyə")
    # ... other speech format tests

def test_nice_duration_numerical(self):
    """Test duration formatting in numerical format."""
    self.assertEqual(nice_duration(1, "az-az", speech=False), "0:01")
    self.assertEqual(nice_duration(61, "az-az", speech=False), "1:01")
    # ... other numerical format tests
test/format_tests/test_format_ru.py (2)

35-47: Consider adding error handling for configuration loading.

While the setup is well-structured, it could benefit from proper error handling for file operations and JSON parsing.

Consider this improvement:

 @classmethod
 def setUpClass(cls):
     # Read date_time_test.json files for test data
     cls.test_config = {}
     p = Path(date_time_format.config_path)
+    if not p.exists():
+        raise ValueError(f"Configuration path {p} does not exist")
     for sub_dir in [x for x in p.iterdir() if x.is_dir()]:
         if (sub_dir / 'date_time_test.json').exists():
-            print("Loading test for " +
-                  str(sub_dir / 'date_time_test.json'))
+            config_file = sub_dir / 'date_time_test.json'
+            try:
+                with config_file.open() as f:
+                    cls.test_config[sub_dir.parts[-1]] = json.loads(f.read())
+            except json.JSONDecodeError as e:
+                raise ValueError(f"Invalid JSON in {config_file}: {e}")
+            except Exception as e:
+                raise ValueError(f"Failed to load {config_file}: {e}")
-            with (sub_dir / 'date_time_test.json').open() as f:
-                cls.test_config[sub_dir.parts[-1]] = json.loads(f.read())

48-166: Consider using parameterized tests for better maintainability.

The time conversion tests are comprehensive but could be more maintainable using parameterization.

Consider using parameterized or pytest.mark.parametrize:

from parameterized import parameterized

class TestNiceDateFormat(unittest.TestCase):
    @parameterized.expand([
        # (datetime_obj, use_24hour, use_ampm, speech, expected)
        (datetime.datetime(2017, 1, 31, 13, 22, 3), False, False, True, "час двадцать два"),
        (datetime.datetime(2017, 1, 31, 13, 22, 3), False, True, True, "час двадцать два дня"),
        # ... add more test cases
    ])
    def test_convert_times(self, dt, use_24hour, use_ampm, speech, expected):
        dt = dt.replace(tzinfo=default_timezone())
        self.assertEqual(
            nice_time(dt, "ru", use_24hour=use_24hour, 
                     use_ampm=use_ampm, speech=speech),
            expected
        )
test/format_tests/test_format_uk.py (5)

34-48: Remove commented debug print statements.

The setup logic is well-structured, but contains commented-out debug print statements that should be removed for cleaner code.

-        # print(p)
-        for sub_dir in [x for x in p.iterdir() if x.is_dir()]:
-            # print(sub_dir)
+        for sub_dir in [x for x in p.iterdir() if x.is_dir()]:
             if (sub_dir / "date_time_test.json").exists():
-                # print(f"Loading test for {sub_dir}/date_time_test.json")
                 with (sub_dir / "date_time_test.json").open() as f:

49-167: Consider using parameterized tests for better maintainability.

While the time conversion tests are comprehensive, they could be more maintainable using parameterized tests. This would reduce code duplication and make it easier to add new test cases.

Example using @pytest.mark.parametrize:

import pytest

@pytest.mark.parametrize("dt,use_24hour,use_ampm,speech,expected", [
    (datetime.datetime(2017, 1, 31, 13, 22, 3), False, False, True, "перша година двадцять два"),
    (datetime.datetime(2017, 1, 31, 13, 22, 3), False, True, True, "перша година дня двадцять два"),
    # ... more test cases
])
def test_nice_time_formats(dt, use_24hour, use_ampm, speech, expected):
    assert nice_time(dt, "uk", use_24hour=use_24hour, 
                    use_ampm=use_ampm, speech=speech) == expected

168-196: Clean up commented code and improve test documentation.

The date formatting tests have two issues to address:

  1. Remove commented debug print statements
  2. The year-long test loop could benefit from a more descriptive comment explaining the test coverage strategy
-        # print(self.test_config[lang]["test_nice_date"].get(str(i)))
         while (self.test_config[lang].get("test_nice_date") and
                self.test_config[lang]["test_nice_date"].get(str(i))):
             p = self.test_config[lang]["test_nice_date"][str(i)]
             dp = ast.literal_eval(p["datetime_param"])
             np = ast.literal_eval(p["now"])
-            # print("Testing for " + lang + " that " + str(dt) +
-            #       " is date " + p["assertEqual"])

197-219: Remove debug print statements.

Similar to the date formatting tests, remove the commented-out debug print statements for cleaner code.

-            # print("Testing for " + lang + " that " + str(dt) +
-            #       " is date time " + p["assertEqual"])
             self.assertEqual(
                 p["assertEqual"],

220-244: Clean up commented code and consider test performance.

  1. Remove commented debug print statements
  2. Consider reducing the year range in the loop test or marking it as a long-running test
-            # print(p)
             dp = ast.literal_eval(p["datetime_param"])
             dt = datetime.datetime(
                 dp[0], dp[1], dp[2], dp[3], dp[4], dp[5],
                 tzinfo=default_timezone())
-            # print("Testing for " + lang + " that " + str(dt) +
-            #           " is year " + p["assertEqual"])
test/parse_tests/test_parse_az.py (3)

28-66: Consider organizing duration tests by complexity.

The duration tests are comprehensive but could be better organized. Consider grouping test cases by:

  1. Basic time units
  2. Fractional durations
  3. Complex multi-unit durations
  4. Sentence structure variations

This would improve maintainability and make it easier to add new test cases.


67-83: Expand test coverage for fractional datetime expressions.

Current tests only cover "half hour" scenarios. Consider adding test cases for:

  • Quarter hour expressions
  • Half day expressions
  • Quarter day expressions

Example test cases to add:

testExtract("bir çərək saat sonra pusu qur",
            "2017-06-27 13:19:00", "pusu qur")
testExtract("yarım gün sonra pusu qur",
            "2017-06-28 01:04:00", "pusu qur")
🧰 Tools
🪛 Ruff

75-75: Undefined name normalize

(F821)


93-93: Remove debug print statement.

The print statement appears to be used for debugging and should be removed from the test code.

-            print(res)
test/format_tests/test_format_ca.py (1)

29-163: Add docstrings to test methods for better documentation.

The test methods would benefit from descriptive docstrings explaining the test scenarios and expected behaviors.

Example for the test_pm method:

     def test_pm(self):
+        """Test afternoon time formatting with various options.
+        
+        Verifies:
+        - Regular time format
+        - AM/PM format
+        - 24-hour format
+        - Speech vs non-speech output
+        - Different Catalan time variants (Watch, Spanish-like, Bell systems)
+        """
         dt = datetime.datetime(2017, 1, 31,
                               13, 22, 3, tzinfo=default_timezone())
test/parse_tests/test_parse_fr.py (6)

29-61: Consider refactoring helper functions to reduce duplication.

The test helper functions share similar logic but differ mainly in their base dates. Consider refactoring them into a single parameterized helper function:

- def extractWithFormat_fr(text):
-     date = datetime(2017, 6, 27, 0, 0, tzinfo=default_timezone())
-     [extractedDate, leftover] = extract_datetime(text, date,
-                                                  lang="fr-fr")
-     extractedDate = extractedDate.strftime("%Y-%m-%d %H:%M:%S")
-     return [extractedDate, leftover]

- def extractWithFormatDate2_fr(text):
-     date = datetime(2017, 6, 30, 17, 0, tzinfo=default_timezone())
-     [extractedDate, leftover] = extract_datetime(text, date,
-                                                  lang="fr-fr")
-     extractedDate = extractedDate.strftime("%Y-%m-%d %H:%M:%S")
-     return [extractedDate, leftover]

- def extractWithFormatNoDate_fr(text):
-     [extractedDate, leftover] = extract_datetime(text, lang="fr-fr")
-     extractedDate = extractedDate.strftime("%Y-%m-%d %H:%M:%S")
-     return [extractedDate, leftover]

+ def extract_with_format_fr(text, base_date=None):
+     """Helper function to extract datetime from French text.
+     
+     Args:
+         text: Input text to parse
+         base_date: Optional base date for relative parsing
+     
+     Returns:
+         tuple: (formatted_date, leftover_text)
+     """
+     if base_date:
+         [extracted_date, leftover] = extract_datetime(text, base_date, 
+                                                     lang="fr-fr")
+     else:
+         [extracted_date, leftover] = extract_datetime(text, lang="fr-fr")
+     
+     if extracted_date:
+         extracted_date = extracted_date.strftime("%Y-%m-%d %H:%M:%S")
+     return [extracted_date, leftover]

This refactoring would:

  • Reduce code duplication
  • Make the code more maintainable
  • Make it easier to add new test cases with different base dates

241-248: Track the disabled timezone test.

The comment indicates a known timezone-related issue that needs to be fixed. This should be tracked to ensure it's not forgotten.

Would you like me to create a GitHub issue to track this timezone-related test and its requirements?


101-104: Remove duplicate test case.

These lines contain identical test cases that should be consolidated.

 testExtract_fr("Quel temps fera-t-il mercredi à 7 heures",
                "2017-06-28 07:00:00", "quel temps fera-t-il")
-testExtract_fr("Quel temps fera-t-il mercredi à 7 heures",
-               "2017-06-28 07:00:00", "quel temps fera-t-il")

249-254: Enhance error case testing.

While the code tests for empty string and invalid hour, consider adding more error cases:

 self.assertEqual(extract_datetime("", lang="fr-fr"), None)
 self.assertEqual(extract_datetime("phrase inutile", lang="fr-fr"),
                  None)
 self.assertEqual(extract_datetime(
     "apprendre à compter à 37 heures", lang="fr-fr"), None)
+# Add tests for more error cases
+self.assertEqual(extract_datetime("le 35ème jour du mois", lang="fr-fr"), None)
+self.assertEqual(extract_datetime("dans -5 jours", lang="fr-fr"), None)
+self.assertEqual(extract_datetime("le 31 février", lang="fr-fr"), None)

255-261: Add more test cases for default time handling.

Consider adding more test cases to thoroughly test the default_time parameter:

 def test_extractdatetime_default_fr(self):
     default = time(9, 0, 0)
     anchor = datetime(2017, 6, 27, 0, 0)
     res = extract_datetime("faire les plantations le 3ème jour de mars",
                            anchor, lang='fr-fr', default_time=default)
     self.assertEqual(default, res[0].time())
+
+    # Test with different default times
+    midnight = time(0, 0, 0)
+    res = extract_datetime("le 15 juillet", anchor, lang='fr-fr',
+                          default_time=midnight)
+    self.assertEqual(midnight, res[0].time())
+
+    # Test when time is specified in text
+    afternoon = time(14, 0, 0)
+    res = extract_datetime("le 15 juillet à 10h", anchor, lang='fr-fr',
+                          default_time=afternoon)
+    self.assertEqual(time(10, 0, 0), res[0].time())

280-286: Track unimplemented fraction parsing features.

The commented tests indicate that fraction parsing for durations is not yet implemented. This should be tracked for future implementation.

Would you like me to create a GitHub issue to track the implementation of fraction parsing in French durations? The issue would cover:

  • Support for "et demi" in duration parsing
  • Handling of fractional minutes/hours
  • Integration with existing duration extraction
test/parse_tests/test_parse_pl.py (3)

64-67: Remove debugging artifacts.

The extractWithFormat function contains:

  1. A print statement marked as TODO for removal
  2. The print statement itself which should be removed
 def extractWithFormat(text):
     date = datetime(2017, 6, 27, 13, 4, tzinfo=default_timezone())  # Tue June 27, 2017 @ 1:04pm
-    print(text)  # TODO Remove me
     [extractedDate, leftover] = extract_datetime(text, date)
🧰 Tools
🪛 Ruff

65-65: Undefined name default_timezone

(F821)


329-331: Remove or utilize unused variables.

The variables morning, evening, and noonish are defined but never used. Either:

  1. Remove them if they're not needed
  2. Add test cases that utilize these time references
-    morning = datetime(2017, 6, 27, 8, 1, 2)
-    evening = datetime(2017, 6, 27, 20, 1, 2)
-    noonish = datetime(2017, 6, 27, 12, 1, 2)
🧰 Tools
🪛 Ruff

329-329: Local variable morning is assigned to but never used

Remove assignment to unused variable morning

(F841)


330-330: Local variable evening is assigned to but never used

Remove assignment to unused variable evening

(F841)


331-331: Local variable noonish is assigned to but never used

Remove assignment to unused variable noonish

(F841)


1-378: Consider enhancing test organization and documentation.

While the test coverage is comprehensive, consider these improvements:

  1. Add docstrings to test classes and methods describing the test scenarios
  2. Group test cases using subtests to improve test output readability
  3. Consider parameterizing similar test cases using @pytest.mark.parametrize if you're using pytest
🧰 Tools
🪛 Ruff

65-65: Undefined name default_timezone

(F821)


72-72: Undefined name normalize

(F821)


329-329: Local variable morning is assigned to but never used

Remove assignment to unused variable morning

(F841)


330-330: Local variable evening is assigned to but never used

Remove assignment to unused variable evening

(F841)


331-331: Local variable noonish is assigned to but never used

Remove assignment to unused variable noonish

(F841)


347-347: Undefined name default_timezone

(F821)


353-353: Undefined name normalize

(F821)

test/parse_tests/test_parse_cs.py (5)

28-66: Add docstring to document test method purpose.

Consider adding a docstring to explain the purpose and scope of the duration extraction tests.

 def test_extract_duration_cs(self):
+    """
+    Test extraction of time durations from Czech text.
+    Verifies parsing of various time units (seconds, minutes, hours, days, weeks)
+    and complex duration phrases.
+    """
     self.assertEqual(extract_duration("10 sekund"),

68-74: Add docstring to document helper function.

The extractWithFormat helper function should be documented to explain its purpose and parameters.

 def extractWithFormat(text):
+    """
+    Helper function to extract and format datetime from text.
+    
+    Args:
+        text (str): Input text containing datetime information
+        
+    Returns:
+        list: [formatted_datetime_str, remaining_text]
+    """
     # Tue June 27, 2017 @ 1:04pm

428-453: Add docstring to document ambiguous time handling.

The test method for ambiguous time handling should be documented to explain its purpose and test scenarios.

 def test_extract_ambiguous_time_cs(self):
+    """
+    Test handling of ambiguous time expressions in Czech.
+    Verifies correct interpretation of time based on context (morning/evening)
+    and handling of invalid or incomplete time expressions.
+    """
     morning = datetime(2017, 6, 27, 8, 1, 2, tzinfo=default_timezone())

454-468: Track incomplete May/may parsing functionality.

The commented test cases indicate incomplete support for handling May/may ambiguity in Czech language parsing.

Would you like me to create a GitHub issue to track the implementation of these test cases and the corresponding functionality?


492-492: Address TODO comment regarding Czech inflection normalization.

The TODO comment indicates a need for better handling of Czech language inflections in the normalization process.

Would you like me to help implement improved normalization for Czech inflections or create a GitHub issue to track this enhancement?

test/parse_tests/test_parse_ru.py (3)

51-55: Document why tests are commented out.

Several test cases are commented out throughout the file. If these are known failures or pending implementation, it should be documented why they're disabled.

Consider adding TODO comments explaining why these tests are skipped, for example:

-# self.assertEqual(extract_duration("разбуди меня через три недели, "
-#                                   "четыреста девяносто семь дней "
-#                                   "и триста 91.6 секунд"),
-#                  (timedelta(weeks=3, days=497, seconds=391.6),
-#                   "разбуди меня через , a"))
+# TODO(JarbasAl): Fix parsing of complex duration combinations
+# self.assertEqual(extract_duration("разбуди меня через три недели, "
+#                                   "четыреста девяносто семь дней "
+#                                   "и триста 91.6 секунд"),
+#                  (timedelta(weeks=3, days=497, seconds=391.6),
+#                   "разбуди меня через , a"))

65-70: Add docstrings to helper functions.

The helper functions extractWithFormat and testExtract would benefit from docstrings explaining their purpose and parameters.

Add docstrings to improve code documentation:

 def extractWithFormat(text):
+    """Helper function to extract datetime with a specific format.
+    
+    Args:
+        text (str): Input text containing date/time information
+        
+    Returns:
+        list: [formatted_date_str, leftover_text]
+    """
     # Tue June 27, 2017 @ 1:04pm
     date = datetime(2017, 6, 27, 13, 4, tzinfo=default_timezone())
     [extractedDate, leftover] = extract_datetime(text, date)
🧰 Tools
🪛 Ruff

68-68: Undefined name default_timezone

(F821)


480-508: Consider grouping related test cases.

The relative datetime tests could be better organized by grouping related test cases (e.g., minutes, hours, days) and using subtests for better test output.

Consider refactoring to use subtests:

 def test_extract_relativedatetime_ru(self):
+    def run_subtest(self, text, expected_date, expected_leftover, msg=None):
+        with self.subTest(text=text):
+            res = extractWithFormat(normalize(text))
+            self.assertEqual(res[0], expected_date, msg or f"for={text}")
+            self.assertEqual(res[1], expected_leftover, msg or f"for={text}")
+
+    # Minutes
+    run_subtest("мы встретимся через 5 минут",
+                "2017-06-27 10:06:02", "мы встретимся")
+    run_subtest("мы встретимся через 1 минуту",
+                "2017-06-27 10:02:02", "мы встретимся")
🧰 Tools
🪛 Ruff

482-482: Undefined name default_timezone

(F821)


488-488: Undefined name normalize

(F821)

test/parse_tests/test_parse_datetime_uk.py (2)

505-535: Address TODO and implement pending test cases.

The test method has:

  1. A TODO comment about spaces error
  2. Commented out test cases that should be implemented

Would you like me to help implement the test cases for handling inputs without spaces between numbers and units (e.g., "5хвилин", "5секунд")?

🧰 Tools
🪛 Ruff

513-513: Undefined name normalize

(F821)


1-538: Complete implementation of commented test cases.

The test file has good coverage for implemented cases but contains many commented-out test cases throughout. This suggests that several test scenarios are pending implementation, particularly for:

  • Time of day parsing (morning/evening)
  • Complex date-time expressions
  • Space-sensitive parsing

Consider:

  1. Creating a tracking issue for implementing the commented test cases
  2. Breaking down the large test file into smaller, focused test files (e.g., separate files for duration, relative time, and ambiguous time tests)
  3. Adding comments explaining why certain test cases are commented out if they are intentionally skipped
🧰 Tools
🪛 Ruff

89-89: Undefined name normalize

(F821)


94-94: Local variable date is assigned to but never used

Remove assignment to unused variable date

(F841)


486-486: Local variable morning is assigned to but never used

Remove assignment to unused variable morning

(F841)


487-487: Local variable evening is assigned to but never used

Remove assignment to unused variable evening

(F841)


488-488: Local variable noonish is assigned to but never used

Remove assignment to unused variable noonish

(F841)


513-513: Undefined name normalize

(F821)

test/parse_tests/test_parse_it.py (2)

43-419: Consider parameterizing test cases to reduce code duplication

The multiple calls to testExtract_it with hard-coded parameters result in duplicated code, which can be refactored using a parameterized approach to improve maintainability and readability.

You can organize the test cases into a list and iterate over them, simplifying the test method. For example:

def test_extractdatetime_it_not_normalized(self):
    test_cases = [
        ('che ore sono adesso', '2018-01-13 13:04:00', 'che ora sono'),
        ('tra due secondi', '2018-01-13 13:04:02', ''),
        # ... include all other test cases here ...
    ]

    def extractWithFormat_it(text):
        date = datetime(2018, 1, 13, 13, 4, tzinfo=default_timezone())
        [extractedDate, leftover] = extract_datetime(text, date, lang='it-it')
        extractedDate = extractedDate.strftime('%Y-%m-%d %H:%M:%S')
        return [extractedDate, leftover]

    for text, expected_date, expected_leftover in test_cases:
        with self.subTest(text=text):
            res = extractWithFormat_it(normalize(text, lang='it-it'))
            self.assertEqual(res[0], expected_date, 'per=' + text)
            self.assertEqual(res[1], expected_leftover, 'per=' + text)

This refactoring reduces code duplication and makes it easier to maintain and extend the test cases.


87-92: Address the TODOs and uncomment the relevant tests

There are commented-out test cases with associated TODO comments. Completing these tests will improve test coverage and ensure that all functionalities are tested thoroughly.

Would you like assistance in implementing these tests or opening a GitHub issue to track this task?

test/parse_tests/test_parse_en.py (2)

156-158: Offer assistance to address TODO for higher numbers in decimal parsing

A TODO comment indicates that handling higher numbers in decimals needs to be fixed in the loop.

Would you like assistance in enhancing the loop to handle higher decimal numbers in test_extractdatetime_year_multiples_en? I can help adjust the test cases and logic accordingly.


270-273: Offer assistance to refine imperfect test due to leftover text

There is a TODO comment noting that the test leaves the time in the remainder, affecting its perfection.

Would you like help in refining the testExtract function to correctly handle the leftover text and improve the test accuracy?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between dd0ed09 and 79732fe.

📒 Files selected for processing (39)
  • .github/workflows/fmtunit_tests.yml (1 hunks)
  • .github/workflows/unit_tests.yml (1 hunks)
  • test/format_tests/test_format.py (1 hunks)
  • test/format_tests/test_format_az.py (1 hunks)
  • test/format_tests/test_format_ca.py (1 hunks)
  • test/format_tests/test_format_cs.py (1 hunks)
  • test/format_tests/test_format_da.py (1 hunks)
  • test/format_tests/test_format_de.py (1 hunks)
  • test/format_tests/test_format_en.py (1 hunks)
  • test/format_tests/test_format_es.py (1 hunks)
  • test/format_tests/test_format_eu.py (1 hunks)
  • test/format_tests/test_format_fa.py (1 hunks)
  • test/format_tests/test_format_fr.py (1 hunks)
  • test/format_tests/test_format_hu.py (1 hunks)
  • test/format_tests/test_format_it.py (1 hunks)
  • test/format_tests/test_format_nl.py (1 hunks)
  • test/format_tests/test_format_pl.py (1 hunks)
  • test/format_tests/test_format_pt.py (1 hunks)
  • test/format_tests/test_format_ru.py (1 hunks)
  • test/format_tests/test_format_sl.py (1 hunks)
  • test/format_tests/test_format_sv.py (1 hunks)
  • test/format_tests/test_format_uk.py (1 hunks)
  • test/parse_tests/test_parse_az.py (1 hunks)
  • test/parse_tests/test_parse_ca.py (1 hunks)
  • test/parse_tests/test_parse_cs.py (1 hunks)
  • test/parse_tests/test_parse_da.py (1 hunks)
  • test/parse_tests/test_parse_datetime_uk.py (1 hunks)
  • test/parse_tests/test_parse_de.py (1 hunks)
  • test/parse_tests/test_parse_en.py (1 hunks)
  • test/parse_tests/test_parse_es.py (1 hunks)
  • test/parse_tests/test_parse_eu.py (1 hunks)
  • test/parse_tests/test_parse_fa.py (1 hunks)
  • test/parse_tests/test_parse_fr.py (1 hunks)
  • test/parse_tests/test_parse_it.py (1 hunks)
  • test/parse_tests/test_parse_nl.py (1 hunks)
  • test/parse_tests/test_parse_pl.py (1 hunks)
  • test/parse_tests/test_parse_pt.py (1 hunks)
  • test/parse_tests/test_parse_ru.py (1 hunks)
  • test/parse_tests/test_parse_sv.py (1 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/fmtunit_tests.yml

45-45: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


47-47: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

.github/workflows/unit_tests.yml

45-45: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


47-47: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 yamllint
.github/workflows/fmtunit_tests.yml

[error] 76-76: no new line character at the end of file

(new-line-at-end-of-file)

🪛 Ruff
test/format_tests/test_format_cs.py

30-30: ovos_date_parser.nice_duration imported but unused

Remove unused import: ovos_date_parser.nice_duration

(F401)

test/parse_tests/test_parse_az.py

75-75: Undefined name normalize

(F821)


92-92: Undefined name normalize

(F821)


248-248: Undefined name normalize

(F821)

test/parse_tests/test_parse_cs.py

76-76: Undefined name normalize

(F821)


478-478: Undefined name normalize

(F821)

test/parse_tests/test_parse_datetime_uk.py

89-89: Undefined name normalize

(F821)


94-94: Local variable date is assigned to but never used

Remove assignment to unused variable date

(F841)


486-486: Local variable morning is assigned to but never used

Remove assignment to unused variable morning

(F841)


487-487: Local variable evening is assigned to but never used

Remove assignment to unused variable evening

(F841)


488-488: Local variable noonish is assigned to but never used

Remove assignment to unused variable noonish

(F841)


513-513: Undefined name normalize

(F821)

test/parse_tests/test_parse_en.py

72-72: Undefined name DAYS_IN_1_MONTH

(F821)


75-75: Undefined name DAYS_IN_1_MONTH

(F821)


78-78: Undefined name DAYS_IN_1_MONTH

(F821)


80-80: Undefined name DAYS_IN_1_YEAR

(F821)


82-82: Undefined name DAYS_IN_1_YEAR

(F821)


84-84: Undefined name DAYS_IN_1_YEAR

(F821)


86-86: Undefined name DAYS_IN_1_YEAR

(F821)


88-88: Undefined name DAYS_IN_1_YEAR

(F821)


90-90: Undefined name DAYS_IN_1_YEAR

(F821)


92-92: Undefined name DAYS_IN_1_YEAR

(F821)


94-94: Undefined name DAYS_IN_1_YEAR

(F821)


96-96: Undefined name DAYS_IN_1_YEAR

(F821)


98-98: Undefined name DAYS_IN_1_YEAR

(F821)


100-100: Undefined name DAYS_IN_1_YEAR

(F821)


106-106: Undefined name extractWithFormat

(F821)


106-106: Undefined name normalize

(F821)


123-123: Undefined name extractWithFormat

(F821)


123-123: Undefined name normalize

(F821)


174-174: Undefined name extractWithFormat

(F821)


174-174: Undefined name normalize

(F821)


497-497: Undefined name normalize

(F821)


726-726: Undefined name extractWithFormat

(F821)


726-726: Undefined name normalize

(F821)


860-860: Undefined name to_local

(F821)


869-869: Undefined name to_local

(F821)


880-880: Undefined name to_local

(F821)


892-892: Undefined name to_local

(F821)


901-901: Undefined name to_local

(F821)


910-910: Undefined name to_local

(F821)


919-919: Undefined name to_local

(F821)


928-928: Undefined name to_local

(F821)


937-937: Undefined name to_local

(F821)


946-946: Undefined name to_local

(F821)


1129-1129: Undefined name normalize

(F821)

test/parse_tests/test_parse_es.py

33-33: Undefined name extract_datetime_es

(F821)


166-166: Undefined name DAYS_IN_1_MONTH

(F821)


169-169: Undefined name DAYS_IN_1_MONTH

(F821)


171-171: Undefined name DAYS_IN_1_YEAR

(F821)


173-173: Undefined name DAYS_IN_1_YEAR

(F821)


175-175: Undefined name DAYS_IN_1_YEAR

(F821)


177-177: Undefined name DAYS_IN_1_YEAR

(F821)


179-179: Undefined name DAYS_IN_1_YEAR

(F821)


181-181: Undefined name DAYS_IN_1_YEAR

(F821)


183-183: Undefined name DAYS_IN_1_YEAR

(F821)


185-185: Undefined name DAYS_IN_1_YEAR

(F821)

test/parse_tests/test_parse_eu.py

24-24: Undefined name now_local

(F821)


27-27: Undefined name extract_datetime_eu

(F821)


31-31: Undefined name extract_datetime

(F821)


33-33: Undefined name default_timezone

(F821)


34-34: Undefined name extract_datetime

(F821)


36-36: Undefined name default_timezone

(F821)


37-37: Undefined name extract_datetime

(F821)


39-39: Undefined name default_timezone

(F821)


40-40: Undefined name extract_datetime

(F821)


42-42: Undefined name default_timezone

(F821)


43-43: Undefined name extract_datetime

(F821)


45-45: Undefined name default_timezone

(F821)


46-46: Undefined name extract_datetime

(F821)


48-48: Undefined name default_timezone

(F821)


49-49: Undefined name extract_datetime

(F821)


51-51: Undefined name default_timezone

(F821)


52-52: Undefined name extract_datetime

(F821)


54-54: Undefined name default_timezone

(F821)


55-55: Undefined name extract_datetime

(F821)


57-57: Undefined name default_timezone

(F821)


58-58: Undefined name extract_datetime

(F821)


60-60: Undefined name default_timezone

(F821)


61-61: Undefined name extract_datetime

(F821)


63-63: Undefined name default_timezone

(F821)


64-64: Undefined name extract_datetime

(F821)


66-66: Undefined name default_timezone

(F821)


67-67: Undefined name extract_datetime

(F821)


69-69: Undefined name default_timezone

(F821)


71-71: Undefined name extract_datetime

(F821)


81-81: Undefined name extract_datetime

(F821)


82-82: Undefined name default_timezone

(F821)


85-85: Undefined name extract_datetime

(F821)


86-86: Undefined name default_timezone

(F821)


87-87: Undefined name extract_datetime

(F821)


89-89: Undefined name default_timezone

(F821)


90-90: Undefined name extract_datetime

(F821)


91-91: Undefined name default_timezone

(F821)


92-92: Undefined name extract_datetime

(F821)


93-93: Undefined name default_timezone

(F821)


94-94: Undefined name extract_datetime

(F821)


95-95: Undefined name default_timezone

(F821)


96-96: Undefined name extract_datetime

(F821)


97-97: Undefined name default_timezone

(F821)


98-98: Undefined name extract_datetime

(F821)


99-99: Undefined name default_timezone

(F821)


100-100: Undefined name extract_datetime

(F821)


101-101: Undefined name default_timezone

(F821)


103-103: Undefined name extract_datetime

(F821)


104-104: Undefined name default_timezone

(F821)


105-105: Undefined name extract_datetime

(F821)


106-106: Undefined name default_timezone

(F821)


108-108: Undefined name extract_datetime

(F821)


109-109: Undefined name default_timezone

(F821)


113-113: Undefined name extract_datetime

(F821)


117-117: Undefined name extract_datetime

(F821)


118-118: Undefined name default_timezone

(F821)

test/parse_tests/test_parse_it.py

32-32: Undefined name default_timezone

(F821)


33-33: Undefined name extract_datetime

(F821)


39-39: Undefined name normalize

(F821)


427-427: Undefined name default_timezone

(F821)


428-428: Undefined name extract_datetime

(F821)


434-434: Undefined name normalize

(F821)


465-465: Undefined name default_timezone

(F821)


466-466: Undefined name default_timezone

(F821)


467-467: Undefined name default_timezone

(F821)


469-469: Undefined name extract_datetime

(F821)


471-471: Undefined name default_timezone

(F821)


473-473: Undefined name extract_datetime

(F821)


475-475: Undefined name default_timezone

(F821)


477-477: Undefined name extract_datetime

(F821)


479-479: Undefined name default_timezone

(F821)


487-487: Undefined name default_timezone

(F821)


488-488: Undefined name extract_datetime

(F821)


494-494: Undefined name normalize

(F821)


515-515: Undefined name extract_datetime

(F821)

test/parse_tests/test_parse_nl.py

33-33: Undefined name LANG

(F821)


104-104: Undefined name LANG

(F821)


109-109: Undefined name LANG

(F821)


112-112: Undefined name LANG

(F821)


114-114: Undefined name LANG

(F821)


116-116: Undefined name LANG

(F821)


118-118: Undefined name LANG

(F821)


120-120: Undefined name LANG

(F821)


122-122: Undefined name LANG

(F821)


124-124: Undefined name LANG

(F821)


126-126: Undefined name LANG

(F821)


128-128: Undefined name LANG

(F821)

test/parse_tests/test_parse_pl.py

65-65: Undefined name default_timezone

(F821)


72-72: Undefined name normalize

(F821)


329-329: Local variable morning is assigned to but never used

Remove assignment to unused variable morning

(F841)


330-330: Local variable evening is assigned to but never used

Remove assignment to unused variable evening

(F841)


331-331: Local variable noonish is assigned to but never used

Remove assignment to unused variable noonish

(F841)


347-347: Undefined name default_timezone

(F821)


353-353: Undefined name normalize

(F821)

test/parse_tests/test_parse_pt.py

31-31: Undefined name default_timezone

(F821)


166-166: Undefined name DAYS_IN_1_MONTH

(F821)


169-169: Undefined name DAYS_IN_1_MONTH

(F821)


172-172: Undefined name DAYS_IN_1_MONTH

(F821)


174-174: Undefined name DAYS_IN_1_YEAR

(F821)


176-176: Undefined name DAYS_IN_1_YEAR

(F821)


178-178: Undefined name DAYS_IN_1_YEAR

(F821)


180-180: Undefined name DAYS_IN_1_YEAR

(F821)


182-182: Undefined name DAYS_IN_1_YEAR

(F821)


184-184: Undefined name DAYS_IN_1_YEAR

(F821)


186-186: Undefined name DAYS_IN_1_YEAR

(F821)


188-188: Undefined name DAYS_IN_1_YEAR

(F821)


190-190: Undefined name DAYS_IN_1_YEAR

(F821)


192-192: Undefined name DAYS_IN_1_YEAR

(F821)


194-194: Undefined name DAYS_IN_1_YEAR

(F821)

test/parse_tests/test_parse_ru.py

68-68: Undefined name default_timezone

(F821)


74-74: Undefined name normalize

(F821)


461-461: Undefined name default_timezone

(F821)


462-462: Undefined name default_timezone

(F821)


463-463: Undefined name default_timezone

(F821)


472-472: Undefined name default_timezone

(F821)


475-475: Undefined name default_timezone

(F821)


478-478: Undefined name default_timezone

(F821)


482-482: Undefined name default_timezone

(F821)


488-488: Undefined name normalize

(F821)

test/parse_tests/test_parse_sv.py

25-25: Undefined name extract_datetime

(F821)


55-55: Undefined name extract_datetime

(F821)


61-61: Undefined name extract_datetime

(F821)


67-67: Undefined name extract_duration

(F821)


71-71: Undefined name extract_duration

(F821)


75-75: Undefined name extract_duration

(F821)


81-81: Undefined name extract_duration

(F821)


85-85: Undefined name extract_duration

(F821)


90-90: Undefined name extract_duration

(F821)


96-96: Undefined name extract_duration

(F821)


99-99: Undefined name extract_duration

(F821)

🔇 Additional comments (61)
.github/workflows/fmtunit_tests.yml (1)

34-38: LGTM! Good use of reusable workflow.

The job effectively leverages an existing workflow for Python build tests.

test/format_tests/test_format_pl.py (2)

1-25: LGTM! Clean imports and proper license header.

The imports are well-organized and the license header is properly included.


104-106: LGTM! Standard unittest main block.

The main block follows the standard unittest pattern.

test/parse_tests/test_parse_nl.py (1)

90-98: Address TODO: Investigate failing test cases.

Several test cases are commented out due to failures. These cases appear to test specific time formats in Dutch (e.g., "8:00 uur", "3 uur 's middags"). This might indicate a gap in the parser's handling of certain Dutch time formats.

Let's check if similar patterns exist in other language test files:

Would you like help investigating why these test cases are failing and implementing the necessary parser improvements?

test/parse_tests/test_parse_da.py (3)

1-24: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license information and includes all necessary imports for the test cases.


129-130: LGTM! Standard unittest main block.

Follows Python testing best practices for standalone test execution.


49-50: Clarify status of commented test cases.

Several test cases are commented out without explanation. These should either be:

  1. Uncommented if they're valid tests
  2. Removed if they're no longer needed
  3. Documented with TODO comments explaining why they're disabled

Also applies to: 100-101, 109-114

test/format_tests/test_format_it.py (2)

1-25: LGTM! License and imports are properly structured.

The file includes appropriate license header and all necessary imports for the test implementation.


136-137: LGTM! Main block is properly implemented.

The main block follows the standard unittest pattern.

test/parse_tests/test_parse_ca.py (3)

1-24: LGTM! Well-structured imports and proper licensing.

The file header is properly licensed and imports are well-organized.


151-152: LGTM! Standard unittest main block.

Properly implements the standard unittest entry point.


1-152: Overall well-implemented test suite with good coverage.

The test file provides comprehensive coverage of Catalan date/time parsing with well-structured test cases. While there are some suggestions for improvement in organization and maintainability, the core functionality is well-tested and properly implemented.

test/format_tests/test_format.py (2)

1-34: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license headers and includes all necessary imports for date/time handling, testing, and configuration.


193-194: LGTM! Standard unittest main block.

The main block correctly enables running the tests directly.

test/format_tests/test_format_en.py (2)

1-25: LGTM! Proper license header and imports.

The license header is complete and imports are well-organized, following the convention of standard library imports first, followed by third-party imports.


149-168: Fix string formatting and verify skip reason.

  1. There's an extra space in the expected string at line 163:
-                         "five days  eighteen hours fifty three minutes twenty seconds")  # nopep8
+                         "five days eighteen hours fifty three minutes twenty seconds")
  1. The test is skipped with "missing code" reason. Please verify if this is still valid or if the implementation is now available.
✅ Verification successful

Let me check the implementation details of the nice_duration function to verify if it's complete.


Let me check if there's an English implementation since the test is for "en" language code.


Test is correctly skipped - English duration formatting is not implemented

The nice_duration() function in __init__.py has implementations for several languages (az, pl, ru, uk) but lacks the English language implementation. The "missing code" skip reason is valid as there's no nice_duration_en() function in the codebase.

  1. The test should remain skipped until the English duration formatting is implemented
  2. Once implemented, fix the extra space in the expected string:
-                         "five days  eighteen hours fifty three minutes twenty seconds")
+                         "five days eighteen hours fifty three minutes twenty seconds")
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if nice_duration implementation exists
rg -A 5 "def nice_duration" 

Length of output: 1843


Script:

#!/bin/bash
# Check nice_duration implementation in __init__.py
rg -A 20 "^def nice_duration" ovos_date_parser/__init__.py

Length of output: 691


Script:

#!/bin/bash
# Check for English implementation
rg -A 30 "^def nice_duration" ovos_date_parser/dates_en.py

Length of output: 60

test/format_tests/test_format_fa.py (1)

1-26: LGTM: Proper license and imports.

The file includes appropriate license header and all necessary imports.

test/format_tests/test_format_da.py (3)

1-24: LGTM! Imports and license are properly structured.

The file includes appropriate license header and all necessary imports.


183-184: LGTM! Standard unittest execution block.

The main execution block is properly implemented.


25-181: Verify coverage of Danish-specific time formats.

While the test coverage is comprehensive, please verify if the following Danish-specific cases are needed:

  • Quarter hour expressions (if Danish has special formats for quarter past/to)
  • Special time expressions unique to Danish culture
  • Formal vs informal time formats
✅ Verification successful

Let me check the parse tests to understand the full Danish time format handling.


Danish time format tests are sufficiently comprehensive

After reviewing both the format and parse tests, the test coverage is complete and includes:

  • All hour formats (1-24)
  • Minutes in various combinations (00, 02, 15, 22, 35, 40, 45, 50, 55)
  • AM/PM indicators ("om morgenen", "om eftermiddagen", "om aftenen", "om natten")
  • Both speech and numeric formats
  • 12/24 hour time formats
  • Special Danish number formations ("femoghalvtres", "toogtyve")
  • Quarter and half hours
  • Default time handling
  • Edge cases like midnight/noon

The initial concern about missing Danish-specific formats is unfounded as the test suite already covers the culturally relevant time expressions in Danish.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other Danish format tests or documentation
rg -g '!test_format_da.py' -l "da-dk" .

# Look for Danish locale configurations
rg -g '*.py' "danish|da-dk" .

Length of output: 6030


Script:

#!/bin/bash
# Check the Danish parse tests for time formats
cat ./test/parse_tests/test_parse_da.py

Length of output: 6666

test/format_tests/test_format_sv.py (3)

1-24: LGTM! Well-organized imports and proper license header.

The code follows good practices with proper license inclusion and organized imports.


184-185: LGTM! Standard unittest main block.

Properly implemented main block for running tests directly.


26-182: Verify complete coverage of time formats.

While the test coverage is extensive, let's verify that all possible time formats are covered:

✅ Verification successful

Let me analyze the test coverage by looking at the implementation details of the Swedish time formatter.


Let me check the test coverage against all possible time formatting scenarios.


Based on the analysis of the implementation and test cases, I can now provide the final response:

Test coverage is comprehensive for Swedish time formatting

The test suite provides thorough coverage of all key time formatting scenarios:

  • Regular times (HH:MM)
  • Quarter hours ("kvart över/i")
  • Half hours ("halv")
  • Minutes before/after the hour
  • 24-hour and 12-hour formats
  • AM/PM variations
  • Speech and display formats
  • Edge cases (1:00 using "ett" vs "en")
  • Special time periods (morning, afternoon)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any missing time format test cases

# Look for other time format test files to compare coverage
echo "Searching for other time format test files..."
fd "test_format_.*\.py" test/format_tests -x rg -l "nice_time.*lang=" {} \;

# Look for nice_time implementation to check supported formats
echo "Checking nice_time implementation for supported formats..."
rg -A 10 "def nice_time" 

Length of output: 16687


Script:

#!/bin/bash
# Check implementation of Swedish time formatter
rg -A 50 "def nice_time_sv" ovos_date_parser/dates_sv.py

# Look for any special cases or format patterns in the implementation
rg "def _" ovos_date_parser/dates_sv.py

Length of output: 1637


Script:

#!/bin/bash
# Check test coverage for different time scenarios
echo "Checking test coverage for different time scenarios..."

# Look for test cases in the Swedish test file
rg "test_convert_times_sv.*def|self\.assertEqual\(nice_time\(" test/format_tests/test_format_sv.py

# Look for the complete implementation to compare against test cases
rg -A 100 "def nice_time_sv" ovos_date_parser/dates_sv.py

Length of output: 7237

test/format_tests/test_format_eu.py (3)

1-23: LGTM: Proper license and imports.

The license header is properly formatted and the imports are appropriate and minimal.


194-196: LGTM: Standard unittest main block.

The main block follows the standard Python unittest pattern.


1-196: Verify complete test coverage.

Let's verify that all parameters and edge cases of the nice_time function are covered in the tests.

✅ Verification successful

Let me generate another script to find the nice_time function definition since it wasn't found in the first attempt.


Based on the results, I can now provide a final response:

Test coverage is comprehensive for the Basque language implementation

The test file provides thorough coverage of the nice_time function parameters and edge cases:

  • Tests both language codes: "eu" and "eu-eu"
  • Covers all function parameters: speech, use_24hour, and use_ampm
  • Tests various time scenarios:
    • Hour boundaries (0:00, 12:00, 13:00)
    • Quarter hours (15, 30, 45 minutes)
    • Special cases like "five to", "quarter to", "half past"
    • Both AM/PM periods
    • 24-hour format variations
  • Includes speech and non-speech output formats
  • Matches the default parameter behavior of the nice_time_eu() implementation found in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any untested nice_time parameters or edge cases

# Find nice_time function definition to check all parameters
echo "Searching for nice_time function definition..."
ast-grep --pattern 'def nice_time($$$)'

# Find all test files to compare test coverage across languages
echo "Checking test coverage across language test files..."
fd "test_format_.+\.py$" test/format_tests/

# Find all test assertions for nice_time to compare coverage
echo "Checking nice_time test assertions..."
rg "nice_time\(.*lang=\"eu" test/format_tests/test_format_eu.py

Length of output: 4812


Script:

#!/bin/bash
# Search for nice_time function in Python files
echo "Searching for nice_time function definition..."
rg "def nice_time" -A 5

# Check test coverage patterns
echo "Analyzing test patterns..."
# Check time ranges
rg "datetime\(.*\)" test/format_tests/test_format_eu.py

# Check parameter combinations
rg "nice_time\(.*\)" test/format_tests/test_format_eu.py | sort | uniq

Length of output: 8819

test/format_tests/test_format_nl.py (3)

1-24: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license information and includes all necessary imports, properly separated between standard library and custom modules.


204-205: LGTM! Standard unittest main block.

Proper implementation of the unittest main block.


27-202: Verify complete coverage of nice_time features.

Let's verify that all features of the nice_time function are covered in the tests.

✅ Verification successful

Test coverage for nice_time function is comprehensive

The test suite demonstrates thorough coverage of the nice_time function's features for Dutch language support:

  • Tests all parameter combinations:

    • Default format (speech=True, use_24hour=False, use_ampm=False)
    • AM/PM format (use_ampm=True)
    • Digital format (speech=False)
    • 24-hour format (use_24hour=True)
    • Combined formats (speech=False with use_ampm, use_24hour with use_ampm)
  • Covers various time scenarios:

    • Hour boundaries (0:02, 1:00, 13:00)
    • Quarter hours (12:15)
    • Minutes past the hour (1:02, 1:22)
    • Minutes to the hour (19:40)
    • Special Dutch phrases ("kwart over", "half", "voor")
    • Different periods (morning, afternoon, evening, night)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for nice_time function definition and usage to ensure all features are tested
echo "Searching for nice_time function definition..."
ast-grep --pattern 'def nice_time($$$)' 

echo "Searching for nice_time parameters in implementation..."
rg "nice_time.*lang=\"nl" -B 2 -A 2

Length of output: 15133

test/format_tests/test_format_hu.py (3)

1-24: LGTM: License and imports are properly structured.

The license header is complete and imports are appropriate for the test requirements.


204-205: LGTM: Main block is properly implemented.

The main block correctly enables standalone test execution.


26-202: Consider adding tests for edge cases.

While the current test coverage is good, consider adding tests for:

  1. Invalid language codes (e.g., "hu" instead of "hu-hu")
  2. None/invalid datetime objects
  3. Boundary conditions (59 minutes, leap seconds, etc.)
  4. Error handling scenarios
test/format_tests/test_format_fr.py (2)

1-24: LGTM! Imports and license are properly structured.

The file includes all necessary imports and has a properly formatted Apache 2.0 license header.


204-205: LGTM! Standard unittest main block.

Properly implements the standard pattern for unittest execution.

test/format_tests/test_format_es.py (2)

1-25: LGTM! License and imports are properly structured.

The license header is complete, and imports are appropriate for the test requirements.


197-198: LGTM! Standard unittest main block.

The main block follows Python best practices for test files.

test/format_tests/test_format_pt.py (3)

1-25: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license information and imports only the necessary components.


205-206: LGTM! Standard unittest main block.

The main block follows the standard pattern for unittest files.


27-206: Verify comprehensive coverage of Portuguese time formats.

While the test coverage is good, let's ensure we haven't missed any Portuguese-specific time formats or edge cases.

✅ Verification successful

Portuguese time format test coverage is comprehensive

The test coverage for Portuguese time formats is thorough and complete, covering:

  • All time periods (madrugada, manhã, tarde, noite)
  • Special times (meia-noite, meio-dia)
  • Both 12-hour and 24-hour formats
  • Minutes past/to the hour
  • Formal and colloquial time expressions
  • AM/PM variations
  • Time with dates
  • Duration parsing
  • Edge cases like "em ponto" (on the dot)

The implementation in dates_pt.py aligns perfectly with the test cases, and there are no TODOs or missing patterns in the Portuguese time handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other Portuguese time format test files and patterns

# Look for other Portuguese test files
echo "Checking for other Portuguese test files..."
fd "test_.*pt.*\.py$"

# Look for Portuguese time format patterns in the main code
echo "Checking for Portuguese time patterns in source..."
rg -l "(?i)portugues|pt_PT|pt_BR" --type py

# Look for TODOs related to Portuguese support
echo "Checking for TODOs related to Portuguese support..."
rg -i "todo.*portug|portug.*todo" --type py

Length of output: 569


Script:

#!/bin/bash
# Let's examine the Portuguese date parsing implementation and test coverage

# Check the implementation of Portuguese date parsing
echo "=== Portuguese date parsing implementation ==="
cat ovos_date_parser/dates_pt.py

# Check the parse tests to compare with format tests
echo -e "\n=== Portuguese parse tests ==="
cat test/parse_tests/test_parse_pt.py

# Look for any time-related constants or patterns
echo -e "\n=== Time-related patterns in Portuguese files ==="
rg "(?i)(hora|tarde|noite|madrugada|meio[- ]dia|meia[- ]noite)" ovos_date_parser/dates_pt.py test/parse_tests/test_parse_pt.py

Length of output: 49777

test/format_tests/test_format_cs.py (1)

1-253: Overall assessment: Well-structured and comprehensive test suite

The test file provides thorough coverage of Czech language date/time formatting functionality with good edge case handling. While there are some code quality improvements suggested above, the core testing functionality is solid and well-implemented.

🧰 Tools
🪛 Ruff

30-30: ovos_date_parser.nice_duration imported but unused

Remove unused import: ovos_date_parser.nice_duration

(F401)

test/format_tests/test_format_de.py (2)

1-24: LGTM: Well-structured file header with appropriate imports.

The file includes proper license headers and all necessary imports for the test cases.


265-266: LGTM: Standard unittest main block.

Properly implements the standard unittest entry point.

test/parse_tests/test_parse_de.py (2)

1-22: LGTM! Clean imports and proper license header.


1-279: Verify test coverage metrics.

While the test cases are comprehensive, it would be good to verify the coverage metrics to ensure we're not missing any edge cases.

✅ Verification successful

After analyzing the implementation file and test files, I can now provide a final response regarding the test coverage verification.

Test coverage appears comprehensive and well-structured

The test file test_parse_de.py provides thorough coverage of the German date/time parsing functionality:

  • Tests cover all major time formats (12/24 hour)
  • Includes tests for various German time expressions and qualifiers (morgens, abends, etc.)
  • Verifies handling of relative dates (morgen, übermorgen, etc.)
  • Tests different date formats and combinations
  • Includes edge cases and error conditions
  • Validates duration parsing with various units
  • Tests default time handling

The test cases align well with the implementation's functionality in dates_de.py, covering all major code paths and edge cases.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for the German parser implementation

# Find the implementation file
implementation_file=$(fd -t f -e py "parser.*de\.py" --full-path)

if [ -n "$implementation_file" ]; then
    # Use coverage.py to generate coverage report
    echo "Implementation file: $implementation_file"
    echo "Running coverage analysis..."
    python3 -m coverage report --include="$implementation_file"
fi

Length of output: 458


Script:

#!/bin/bash
# Let's analyze the test coverage by examining the implementation file directly
# and comparing it with test cases

# First, let's look at the implementation file
echo "=== Implementation file content ==="
cat ovos_date_parser/dates_de.py

# Then, let's find all test files related to German date parsing
echo -e "\n=== Related test files ==="
fd -t f -e py "test.*de\.py"

Length of output: 32499

test/format_tests/test_format_ru.py (3)

1-32: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license information and includes all necessary imports using modern Python practices (e.g., pathlib).


1-268: Overall well-structured test suite with room for improvement.

The test suite provides comprehensive coverage of Russian date/time formatting. Consider the suggested improvements for:

  1. Error handling in configuration loading
  2. Test parameterization for better maintainability
  3. Reduction of code duplication in date-related tests
  4. Resolution of commented test cases

These changes would make the test suite more robust and easier to maintain.


243-262: Clarify status of commented test cases.

There are several commented test cases for speech=False scenarios. These should either be implemented, removed, or documented with a reason for being disabled.

Let's check if these features are tracked elsewhere:

Would you like help implementing the commented test cases or creating an issue to track this work?

✅ Verification successful

Commented test cases reflect unimplemented feature

The commented test cases for speech=False are correctly disabled because the feature is explicitly not implemented in the Russian language handler. The nice_duration_ru() function raises NotImplementedError when speech=False.

  • ovos_date_parser/dates_ru.py: Contains explicit raise NotImplementedError for speech=False case

These test cases should remain commented out until the non-speech duration formatting is implemented for Russian language support.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related issues or TODOs
rg -i "speech.*false.*duration" 
rg -i "TODO.*duration.*speech"

Length of output: 66


Script:

#!/bin/bash
# Let's check the implementation of nice_duration function to understand if speech=False is supported
rg -l "nice_duration" --type py

# Then examine the implementation files
rg "def nice_duration" -A 10

Length of output: 3638


Script:

#!/bin/bash
# Let's check the implementation of nice_duration_ru to see if speech=False is actually implemented
rg -A 50 "def nice_duration_ru" ovos_date_parser/dates_ru.py

Length of output: 1366

test/format_tests/test_format_uk.py (1)

1-33: LGTM! Well-structured imports and proper license header.

The file starts with appropriate license information and includes all necessary imports using modern Python practices.

test/parse_tests/test_parse_az.py (1)

1-263: Overall test coverage looks good!

The test file provides comprehensive coverage for Azerbaijani language date/time parsing with well-structured test cases. While there are some suggestions for improvements, the core functionality is well tested.

🧰 Tools
🪛 Ruff

75-75: Undefined name normalize

(F821)


92-92: Undefined name normalize

(F821)


248-248: Undefined name normalize

(F821)

test/format_tests/test_format_ca.py (2)

1-26: LGTM! Well-structured imports and proper licensing.

The file header includes appropriate license information and necessary imports for the test suite.


29-286: Verify test coverage for edge cases.

The test suite covers main scenarios but should verify additional edge cases:

  • Invalid time inputs
  • Boundary conditions (e.g., 23:59, 00:00)
  • Special time expressions in Catalan

Let's analyze the test coverage:

test/parse_tests/test_parse_pl.py (2)

26-62: LGTM! Comprehensive duration extraction test cases.

The test cases thoroughly cover various duration formats in Polish, including:

  • Basic time units (seconds, minutes, hours, days, weeks)
  • Decimal numbers
  • Complex phrases
  • Different number formats

357-374: LGTM! Well-structured relative datetime test cases.

The test cases provide comprehensive coverage for relative time expressions in Polish, including:

  • Minutes, seconds, and hours
  • Different number formats and spacing variations
  • Single and plural time units
test/parse_tests/test_parse_cs.py (1)

1-24: LGTM!

The license header and imports are properly structured.

test/parse_tests/test_parse_ru.py (1)

1-511: Overall well-structured test suite with good coverage.

The test file provides comprehensive coverage of Russian date/time parsing with good attention to language-specific nuances. The test cases are thorough and well-organized.

A few suggestions for improvement:

  1. Add missing imports
  2. Document commented-out tests
  3. Add helper function docstrings
  4. Consider using subtests for better organization

But otherwise, the implementation is solid and ready for use.

🧰 Tools
🪛 Ruff

68-68: Undefined name default_timezone

(F821)


74-74: Undefined name normalize

(F821)


461-461: Undefined name default_timezone

(F821)


462-462: Undefined name default_timezone

(F821)


463-463: Undefined name default_timezone

(F821)


472-472: Undefined name default_timezone

(F821)


475-475: Undefined name default_timezone

(F821)


478-478: Undefined name default_timezone

(F821)


482-482: Undefined name default_timezone

(F821)


488-488: Undefined name normalize

(F821)

test/parse_tests/test_parse_datetime_uk.py (1)

28-84: Well-structured duration extraction tests!

The test cases are comprehensive, covering:

  • Various time units (days, seconds, minutes, hours, weeks)
  • Decimal values and complex phrases
  • Edge cases with leftover text
test/parse_tests/test_parse_it.py (1)

464-479: Undefined names 'default_timezone' and 'extract_datetime'

The functions default_timezone and extract_datetime are used but not imported, which will result in NameErrors when the code is executed.

Refer to the earlier comment regarding missing imports to resolve this issue.

🧰 Tools
🪛 Ruff

465-465: Undefined name default_timezone

(F821)


466-466: Undefined name default_timezone

(F821)


467-467: Undefined name default_timezone

(F821)


469-469: Undefined name extract_datetime

(F821)


471-471: Undefined name default_timezone

(F821)


473-473: Undefined name extract_datetime

(F821)


475-475: Undefined name default_timezone

(F821)


477-477: Undefined name extract_datetime

(F821)


479-479: Undefined name default_timezone

(F821)

test/parse_tests/test_parse_en.py (4)

121-151: Undefined functions extractWithFormat and normalize in test_extractdatetime_year_multiples_en

🧰 Tools
🪛 Ruff

123-123: Undefined name extractWithFormat

(F821)


123-123: Undefined name normalize

(F821)


172-176: Undefined functions extractWithFormat and normalize in test_extractdatetime_en

🧰 Tools
🪛 Ruff

174-174: Undefined name extractWithFormat

(F821)


174-174: Undefined name normalize

(F821)


892-953: Undefined function to_local in test_extract_last_int_dateunit_en

🧰 Tools
🪛 Ruff

892-892: Undefined name to_local

(F821)


901-901: Undefined name to_local

(F821)


910-910: Undefined name to_local

(F821)


919-919: Undefined name to_local

(F821)


928-928: Undefined name to_local

(F821)


937-937: Undefined name to_local

(F821)


946-946: Undefined name to_local

(F821)


806-810: Skipped test due to normalization limitations

The test test_extract_within_the_en is skipped with a note about disambiguation issues between 'a' and 'the' due to normalization.

Would you like assistance in addressing this issue? I can help investigate ways to improve normalization to handle this disambiguation.

Alternatively, to verify the current behavior, you can run the following script:

This will help determine if the normalization process can be adjusted.

✅ Verification successful

Let me check the test implementation and normalization logic to better understand the issue.


Let me check the normalization implementation in the English date parser to understand how it handles these cases.


Let me check the actual test cases and their implementations to understand how the time extraction works for "within" phrases.


Skipped test is correctly addressing a semantic difference

The test is correctly skipped as it highlights a genuine semantic difference between "within a" and "within the" that the current normalization can't handle:

  • "within a month" adds a duration (timedelta of 30 days)
  • "within the month" means before the start of next month

This is not a bug but a legitimate language processing challenge that requires preserving the article ("a" vs "the") during normalization to maintain these distinct meanings.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for usage of 'within a' and 'within the' in the codebase.

# Find occurrences of 'within a'
rg -o 'within a \w+' test/

# Find occurrences of 'within the'
rg -o 'within the \w+' test/

Length of output: 1208


Script:

#!/bin/bash
# Get the test implementation and surrounding context
rg -B 5 -A 10 "test_extract_within_the_en" test/parse_tests/test_parse_en.py

# Check for normalization related code
rg -l "normalize" .

Length of output: 1511


Script:

#!/bin/bash
# Check the normalization implementation in dates_en.py
rg -B 2 -A 5 "def normalize" ovos_date_parser/dates_en.py

# Look for handling of "within" phrases
rg -B 2 -A 5 "within" ovos_date_parser/dates_en.py

Length of output: 949


Script:

#!/bin/bash
# Get all test cases related to "within" time extraction
rg -B 2 -A 2 "extract_datetime.*within" test/parse_tests/test_parse_en.py

# Check the extract_datetime implementation
rg -B 5 -A 10 "def extract_datetime" ovos_date_parser/dates_en.py

Length of output: 4524

Comment on lines +16 to +18
import unittest
from datetime import datetime, time, timedelta

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing imports for core functions.

The test file is missing imports for the core functions being tested: extract_datetime and extract_duration. These should be imported from the main package.

Add the following import:

import unittest
from datetime import datetime, time, timedelta
+ from ovos_date_parser import extract_datetime, extract_duration
📝 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
import unittest
from datetime import datetime, time, timedelta
import unittest
from datetime import datetime, time, timedelta
from ovos_date_parser import extract_datetime, extract_duration

Comment on lines +94 to +109
# testExtract("ساعت بیست و دو و چهل و پنج دقیقه بهم یادآوری کن",
# "2017-06-27 22:45:00", "بهم یادآوری کن")
testExtract("هوای جمعه صبح چطوره؟",
"2017-06-30 08:00:00", "هوای چطوره؟")
testExtract("هوای فردا چطوره؟",
"2017-06-28 00:00:00", "هوای چطوره؟")
testExtract("هوای امروز بعد از ظهر چطوره؟",
"2017-06-27 15:00:00", "هوای چطوره؟")
testExtract("یادم بنداز که هشت هفته و دو روز دیگه به مادرم زنگ بزنم",
"2017-08-24 00:00:00", "یادم بنداز که به مادرم زنگ بزنم")
# testExtract("یادم بنداز که دوازده مرداد به مادرم زنگ بزنم",
# "2017-08-03 00:00:00", "یادم بنداز که به مادرم زنگ بزنم")
# testExtract("یادم بنداز که ساعت هفت به مادرم زنگ بزنم",
# "2017-06-28 07:00:00", "یادم بنداز که به مادرم زنگ بزنم")
# testExtract("یادم بنداز که فردا ساعت بیست و دو به مادرم زنگ بزنم",
# "2017-06-28 22:00:00", "یادم بنداز که به مادرم زنگ بزنم")
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 7, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Uncomment and fix the Persian time parsing test cases

The commented-out test cases appear to be testing specific Persian time parsing features:

  • Time with hours and minutes ("22:45")
  • Persian month name ("مرداد")
  • Specific hour references ("ساعت هفت", "ساعت بیست و دو")
  • Combined date-time expressions

These test cases should be fixed and uncommented as they cover important Persian time parsing functionality that isn't tested by other active test cases. The presence of a TODO comment at the end of the file suggests this is a known work in progress.

🔗 Analysis chain

Address commented-out test cases.

Several test cases are commented out, suggesting incomplete or failing functionality. These should either be:

  1. Fixed and uncommented if they represent valid test cases
  2. Removed if they're no longer relevant
  3. Documented with a comment explaining why they're disabled

Let's check if these are known issues:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for related issues or discussions about Persian time parsing
rg -i "persian|farsi|fa.*time" .github/

Length of output: 41


Script:

#!/bin/bash
# Let's search for any related issues or discussions in the entire codebase
rg -i "persian|farsi" -g '!*.pyc' -g '!*.txt'

# Also check for any documentation about these test cases
rg -i "persian.*test|farsi.*test" -g '!*.pyc'

# Look for any TODOs or FIXMEs related to Persian/Farsi parsing
rg -i "TODO.*\b(persian|farsi)\b|FIXME.*\b(persian|farsi)\b" -g '!*.pyc'

Length of output: 170


Script:

#!/bin/bash
# Let's look at the test file content and surrounding context
cat test/parse_tests/test_parse_fa.py

# Also check for any similar patterns in other test files
fd test_parse_ --exec head -n 1 {}

Length of output: 5639

Copy link
Member Author

Choose a reason for hiding this comment

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

open an issue to track this

Copy link
Contributor

Choose a reason for hiding this comment

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

@JarbasAl, I've opened a new GitHub issue to track this: #54

🔗 Analysis chain

@JarbasAl, I'll open a new GitHub issue to track this.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
gh issue create \
  --title "Uncomment and fix Persian time parsing test cases" \
  --body "The commented-out test cases in \`test/parse_tests/test_parse_fa.py\` should be fixed and uncommented. These test cases cover important Persian time parsing functionality that are not tested by other active test cases. See the discussion in PR #51: https://github.com/OpenVoiceOS/ovos-date-parser/pull/51#discussion_r1832093961.\n\nRequested by @JarbasAl." \
  --assignee JarbasAl

Length of output: 518

Comment on lines +17 to +24
import unittest
from datetime import datetime, time, timedelta

from ovos_config.locale import get_default_tz as default_timezone

from ovos_date_parser import (
extract_duration, extract_datetime
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing LANG constant definition.

The test file is missing the definition of the LANG constant which is used throughout the test cases. This causes multiple undefined variable errors.

Add this line after the imports:

from ovos_date_parser import (
    extract_duration, extract_datetime
)

+ LANG = "nl"  # Dutch language code
📝 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
import unittest
from datetime import datetime, time, timedelta
from ovos_config.locale import get_default_tz as default_timezone
from ovos_date_parser import (
extract_duration, extract_datetime
)
import unittest
from datetime import datetime, time, timedelta
from ovos_config.locale import get_default_tz as default_timezone
from ovos_date_parser import (
extract_duration, extract_datetime
)
LANG = "nl" # Dutch language code

Comment on lines +1129 to +1131
res = extractWithFormat(normalize(text))
self.assertEqual(res[0], expected_date, "for=" + text)
self.assertEqual(res[1], expected_leftover, "for=" + text)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined function normalize in test_extract_relativedatetime_en

The function normalize is used but not defined or imported, causing potential NameError exceptions.

Ensure normalize is properly imported:

 from ovos_date_parser import (
     extract_duration, extract_datetime
+    , normalize
 )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff

1129-1129: Undefined name normalize

(F821)

Comment on lines +497 to +499
res = extractWithFormat(normalize(text))
self.assertEqual(res[0], expected_date, "for=" + text)
self.assertEqual(res[1], expected_leftover, "for=" + text)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined function normalize in test_extractdatetime_with_default_time_en

The function normalize is used but not defined or imported, which will cause a NameError during execution.

Ensure normalize is imported from the appropriate module:

 from ovos_date_parser import (
     extract_duration, extract_datetime
+    , normalize
 )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff

497-497: Undefined name normalize

(F821)

Comment on lines +104 to +120
def test_extractdatetime_fractions_en(self):
def testExtract(text, expected_date, expected_leftover):
res = extractWithFormat(normalize(text))
self.assertEqual(res[0], expected_date, "for=" + text)
self.assertEqual(res[1], expected_leftover, "for=" + text)

testExtract("Set the ambush for half an hour",
"2017-06-27 13:34:00", "set ambush")
testExtract("remind me to call mom in half an hour",
"2017-06-27 13:34:00", "remind me to call mom")
testExtract("remind me to call mom in a half hour",
"2017-06-27 13:34:00", "remind me to call mom")
testExtract("remind me to call mom in a quarter hour",
"2017-06-27 13:19:00", "remind me to call mom")
testExtract("remind me to call mom in a quarter of an hour",
"2017-06-27 13:19:00", "remind me to call mom")

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined functions extractWithFormat and normalize in test_extractdatetime_fractions_en

The functions extractWithFormat and normalize are used but not defined or imported, leading to NameError exceptions during test execution.

Ensure these functions are properly imported. If they are part of ovos_date_parser, update the import statement:

 from ovos_date_parser import (
     extract_duration, extract_datetime
+    , normalize, extractWithFormat
 )

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff

106-106: Undefined name extractWithFormat

(F821)


106-106: Undefined name normalize

(F821)

Comment on lines +859 to +887
for i in range(1, 1500):
expected = to_local(dt - timedelta(seconds=i))
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the past {i} seconds', dt)[0],
expected)
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the last {i} seconds', dt)[0],
expected)

for i in range(1, 1500):
expected = to_local(dt - timedelta(minutes=i))
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the past {i} minutes', dt)[0],
expected)
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the last {i} minutes', dt)[0],
expected)

# this is also testing conflicts with military time,
# the parser ignores 100 <= N <= 2400 unless there is a past_marker word
for i in range(1, 1500):
expected = to_local(dt - timedelta(hours=i))
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the past {i} hours', dt)[0],
expected)
self.assertEqual(
extract_datetime(f'i wrote a lot of unittests in the last {i} hours', dt)[0],
expected)

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined function to_local in test_extract_last_int_timeunit_en

The function to_local is used but not defined or imported, which will cause NameError exceptions during test execution.

Define to_local or import it if it's part of another module. For example:

+ def to_local(dt):
+     return dt.astimezone(default_timezone())

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 Ruff

860-860: Undefined name to_local

(F821)


869-869: Undefined name to_local

(F821)


880-880: Undefined name to_local

(F821)

Comment on lines +71 to +101
self.assertEqual(extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))
self.assertEqual(
extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))

self.assertEqual(extract_duration("3 months"),
(timedelta(days=DAYS_IN_1_MONTH * 3), ""))
self.assertEqual(extract_duration("a year"),
(timedelta(days=DAYS_IN_1_YEAR), ""))
self.assertEqual(extract_duration("1 year"),
(timedelta(days=DAYS_IN_1_YEAR * 1), ""))
self.assertEqual(extract_duration("5 years"),
(timedelta(days=DAYS_IN_1_YEAR * 5), ""))
self.assertEqual(extract_duration("a decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("1 decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("5 decades"),
(timedelta(days=DAYS_IN_1_YEAR * 10 * 5), ""))
self.assertEqual(extract_duration("1 century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("a century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("5 centuries"),
(timedelta(days=DAYS_IN_1_YEAR * 100 * 5), ""))
self.assertEqual(extract_duration("1 millennium"),
(timedelta(days=DAYS_IN_1_YEAR * 1000), ""))
self.assertEqual(extract_duration("5 millenniums"),
(timedelta(days=DAYS_IN_1_YEAR * 1000 * 5), ""))

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined constants DAYS_IN_1_MONTH and DAYS_IN_1_YEAR in test_non_std_units

The constants DAYS_IN_1_MONTH and DAYS_IN_1_YEAR are used but not defined, which will cause NameError exceptions when running the tests.

Apply this diff to define the missing constants at the beginning of the file:

+ DAYS_IN_1_MONTH = 30  # Average number of days in a month
+ DAYS_IN_1_YEAR = 365  # Number of days in a non-leap year

Alternatively, import these constants if they are defined in another module.

📝 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
self.assertEqual(extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))
self.assertEqual(
extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))
self.assertEqual(extract_duration("3 months"),
(timedelta(days=DAYS_IN_1_MONTH * 3), ""))
self.assertEqual(extract_duration("a year"),
(timedelta(days=DAYS_IN_1_YEAR), ""))
self.assertEqual(extract_duration("1 year"),
(timedelta(days=DAYS_IN_1_YEAR * 1), ""))
self.assertEqual(extract_duration("5 years"),
(timedelta(days=DAYS_IN_1_YEAR * 5), ""))
self.assertEqual(extract_duration("a decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("1 decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("5 decades"),
(timedelta(days=DAYS_IN_1_YEAR * 10 * 5), ""))
self.assertEqual(extract_duration("1 century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("a century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("5 centuries"),
(timedelta(days=DAYS_IN_1_YEAR * 100 * 5), ""))
self.assertEqual(extract_duration("1 millennium"),
(timedelta(days=DAYS_IN_1_YEAR * 1000), ""))
self.assertEqual(extract_duration("5 millenniums"),
(timedelta(days=DAYS_IN_1_YEAR * 1000 * 5), ""))
DAYS_IN_1_MONTH = 30 # Average number of days in a month
DAYS_IN_1_YEAR = 365 # Number of days in a non-leap year
self.assertEqual(extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))
self.assertEqual(
extract_duration("1 month"),
(timedelta(days=DAYS_IN_1_MONTH), ""))
self.assertEqual(extract_duration("3 months"),
(timedelta(days=DAYS_IN_1_MONTH * 3), ""))
self.assertEqual(extract_duration("a year"),
(timedelta(days=DAYS_IN_1_YEAR), ""))
self.assertEqual(extract_duration("1 year"),
(timedelta(days=DAYS_IN_1_YEAR * 1), ""))
self.assertEqual(extract_duration("5 years"),
(timedelta(days=DAYS_IN_1_YEAR * 5), ""))
self.assertEqual(extract_duration("a decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("1 decade"),
(timedelta(days=DAYS_IN_1_YEAR * 10), ""))
self.assertEqual(extract_duration("5 decades"),
(timedelta(days=DAYS_IN_1_YEAR * 10 * 5), ""))
self.assertEqual(extract_duration("1 century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("a century"),
(timedelta(days=DAYS_IN_1_YEAR * 100), ""))
self.assertEqual(extract_duration("5 centuries"),
(timedelta(days=DAYS_IN_1_YEAR * 100 * 5), ""))
self.assertEqual(extract_duration("1 millennium"),
(timedelta(days=DAYS_IN_1_YEAR * 1000), ""))
self.assertEqual(extract_duration("5 millenniums"),
(timedelta(days=DAYS_IN_1_YEAR * 1000 * 5), ""))
🧰 Tools
🪛 Ruff

72-72: Undefined name DAYS_IN_1_MONTH

(F821)


75-75: Undefined name DAYS_IN_1_MONTH

(F821)


78-78: Undefined name DAYS_IN_1_MONTH

(F821)


80-80: Undefined name DAYS_IN_1_YEAR

(F821)


82-82: Undefined name DAYS_IN_1_YEAR

(F821)


84-84: Undefined name DAYS_IN_1_YEAR

(F821)


86-86: Undefined name DAYS_IN_1_YEAR

(F821)


88-88: Undefined name DAYS_IN_1_YEAR

(F821)


90-90: Undefined name DAYS_IN_1_YEAR

(F821)


92-92: Undefined name DAYS_IN_1_YEAR

(F821)


94-94: Undefined name DAYS_IN_1_YEAR

(F821)


96-96: Undefined name DAYS_IN_1_YEAR

(F821)


98-98: Undefined name DAYS_IN_1_YEAR

(F821)


100-100: Undefined name DAYS_IN_1_YEAR

(F821)

JarbasAl and others added 3 commits November 7, 2024 06:00
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant