Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

%Y-%W-%w parsing for 2019 #961

Closed
Bidek56 opened this issue Feb 13, 2023 · 7 comments
Closed

%Y-%W-%w parsing for 2019 #961

Bidek56 opened this issue Feb 13, 2023 · 7 comments

Comments

@Bidek56
Copy link

Bidek56 commented Feb 13, 2023

let date = NaiveDate::parse_from_str("2020-01-0", "%Y-%W-%w")?;
Returns: 2020-01-12
But:
let date = NaiveDate::parse_from_str("2019-01-0", "%Y-%W-%w")?;
Returns: Error: ParseError(Impossible)

Any reasons? Thx

@djc
Copy link
Member

djc commented Feb 14, 2023

Okay, there seems to be a bug in the logic for verify_ordinal that you've hit. My guess is the logic for the week_from_sun verification was copied to week_from_mon, but the latter version is incorrect in this case (potentially only for the particular case of years starting on a Tuesday?) with an off-by-one error.

I can't quite work out the correct logic right now, please submit a PR if you can (and add the failing case as a test case).

@esheppa
Copy link
Collaborator

esheppa commented Feb 14, 2023

I think the fix is the same as from https://github.com/chronotope/chrono/pull/921/files - I'll push a commit to that PR

@FObersteiner
Copy link

a related question: why is %w needed here? The docs don't say so explicitly and for example %Y-%m also works without needing a day, it just defaults to the first day of the month.

I could make a separate issue if this is too much off-topic here ;-)

@djc
Copy link
Member

djc commented Feb 14, 2023

%W identifies a week, but not a date, so you can't parse a date out of it.

@FObersteiner
Copy link

@djc that was my first thought as well. But if you take %m for example, similarly that identifies a month and not a date. Nevertheless, you can parse 2019-01 with %Y-%m to a full date. I don't see why this shouldn't work technically with %W. Just default to the first day of the week. Or, if there is a clear reason why not to do so, be more explicit in the docs.

@djc
Copy link
Member

djc commented Feb 15, 2023

I guess if someone wants to contribute code to allow leaving off %w I'd be open to merging that.

@pitdicker
Copy link
Collaborator

Fixed in #966.

Allowing default/missing values when parsing is a duplicate of #528 and a couple of other issues.

@djc djc closed this as completed Jun 7, 2023
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

No branches or pull requests

5 participants