Skip to content

Conversation

ShashwatAgrawal20
Copy link
Contributor

@ShashwatAgrawal20 ShashwatAgrawal20 commented Feb 9, 2025

@ShashwatAgrawal20
Copy link
Contributor Author

Not sure if I'm supposed to be adding the tests in pandas/tests/tslibs/test_strptime.py or not.

@ShashwatAgrawal20 ShashwatAgrawal20 marked this pull request as ready for review February 10, 2025 10:44
@mroeschke
Copy link
Member

Not sure if I'm supposed to be adding the tests in pandas/tests/tslibs/test_strptime.py or not.

You can add a test in test_to_datetime.py with the example from the original issue

@mroeschke mroeschke added the Datetime Datetime data dtype label Feb 10, 2025
Comment on lines 822 to 826
@pytest.mark.parametrize("errors", ["raise"])
def test_invalid_iso_week_53(self, msg, s, _format, errors):
# See GH#60885
with pytest.raises(ValueError, match=msg):
to_datetime(s, format=_format, errors=errors)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@pytest.mark.parametrize("errors", ["raise"])
def test_invalid_iso_week_53(self, msg, s, _format, errors):
# See GH#60885
with pytest.raises(ValueError, match=msg):
to_datetime(s, format=_format, errors=errors)
def test_invalid_iso_week_53(self, msg, s, _format):
# See GH#60885
with pytest.raises(ValueError, match=msg):
to_datetime(s, format=_format)

errors="raise" is the default so we don't need to specify it here

@mroeschke mroeschke added this to the 3.0 milestone Feb 12, 2025
@mroeschke mroeschke merged commit b67668e into pandas-dev:main Feb 12, 2025
42 checks passed
@mroeschke
Copy link
Member

Thanks @ShashwatAgrawal20

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

Labels

Datetime Datetime data dtype

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: pandas to_datetime() returns incorrect isoweek conversion in week 53 when only 52 weeks exist

2 participants