Skip to content

Conversation

@victorlin
Copy link
Member

Note

Based on #1913

Description of proposed changes

This fixes a bug where incomplete dates such as --max-date 2018 would not be inclusive, since that had previously resolved to --max-date 2018-01-01.

numeric_date() and get_numerical_date_from_value() are two separate functions that serve similar purposes: converting some date value to a numeric date. The latter has had more recent developments, and crucially will return a date range for ambiguous dates. This is desired for min/max date filters, so I've switched to it.

Related issue(s)

Closes #893

Checklist

  • Merge base PR
  • Automated checks pass
  • Check if you need to add a changelog message
  • Check if you need to add tests
  • Check if you need to update docs

This puts the function on par with numeric_date() for string types.
This fixes a bug where incomplete dates such as --max-date 2018 would
not be inclusive, since that had previously resolved to --max-date
2018-01-01.

numeric_date() and get_numerical_date_from_value() are two separate
functions that serve similar purposes: converting some date value to a
numeric date. The latter has had more recent developments, and crucially
will return a date range for ambiguous dates. This is desired for
min/max date filters, so I've switched to it.
@victorlin victorlin self-assigned this Oct 22, 2025
@victorlin victorlin linked an issue Oct 22, 2025 that may be closed by this pull request
Copy link
Member Author

Choose a reason for hiding this comment

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

Some test matrix jobs are failing at tests/filter/test_relative_dates.py:

tests/filter/test_relative_dates.py:152: in test_filter_relative_dates
    assert output_sorted == output_sorted_expected
E   AssertionError: assert ['SEQ_1', 'SEQ_2', 'SEQ_3'] == ['SEQ_2', 'SEQ_3']
E     
E     At index 0 diff: 'SEQ_1' != 'SEQ_2'
E     Left contains one more item: 'SEQ_3'
E     
E     Full diff:
E       [
E     +     'SEQ_1',
E           'SEQ_2',
E           'SEQ_3',
E       ]
        argparse_params = '--min-date 1D'
…
================== 6 failed, 569 passed, 2 warnings in 14.95s ==================

This needs investigating. To narrow things down a bit, I noticed it happens in test (python=3.10 biopython=1.80 numpy=latest) and not test (python=3.10 biopython=1.80 numpy=1.26.4). I'm unable to reproduce locally with (python=3.13 biopython=1.85 numpy=2.3.2), so it's not just the numpy version.

(unrelated to this file, just needed to start a thread)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--max-date with year only is not inclusive

2 participants