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

Fix regex for time value #512

Merged
merged 6 commits into from Feb 6, 2023
Merged

Fix regex for time value #512

merged 6 commits into from Feb 6, 2023

Conversation

eloiferrer
Copy link
Contributor

Time values with month = 00 or day = 00 (e.g. +2023-00-00T00:00:00Z, +2023-02-00T00:00:00Z) are also allowed and should not raise an error.

@LeMyst
Copy link
Owner

LeMyst commented Feb 2, 2023

Hello @eloiferrer ,
Thank you for your PR.
I tried to create an item with a Time claim at "+2023-00-00T00:00:00Z" on test.wikidata.org and receive a 'Malformed input: +2023-00-00T00:00:00Z' as a result.

Can you share your Mediawiki and Wikibase version and the, if your wikibase is public, an item with a claim like this?

Thank you,

@eloiferrer
Copy link
Contributor Author

eloiferrer commented Feb 3, 2023

Hi,

We are using Mediawiki 1.39.0-wmf.22, but I cannot yet point you to a public item using this format, although it does work for me locally.

According to the examples shown here (https://www.wikidata.org/wiki/Help:Dates#Precision) this should be possible, so I am confused about the Malformed Input error.

The item https://www.wikidata.org/wiki/Q301, for instance, has a date of birth equal to +1541-00-00T00:00:00Z. When I try to input +2023-00-00T00:00:00Z on test.wikidata.org with 'Date of birth' or 'Publication Date' I don't get an error. Maybe the Malformed Input refers only to the specific property you are using, or to some contradiction with the precision parameter?

@LeMyst
Copy link
Owner

LeMyst commented Feb 3, 2023

After more tests, you need to add the correction precision if you omit the day or the month. (Wikibase web UI do this automatically)

I'll try to add more logic into the test.

@LeMyst
Copy link
Owner

LeMyst commented Feb 3, 2023

What do you think about the latest commits?

@eloiferrer
Copy link
Contributor Author

eloiferrer commented Feb 3, 2023

I think it definitely works and I can work with that. But since the precision is implicit in the format, I think it would be better to add the logic that directly derives the right precision when the pattern is matched (if no precision is given by the user). Basically to avoid having to explicitly introduce the precision everytime that it is not days, as by default.

I mean, if no precision is given by the user but match(YYYY-MM-00T00:00:00Z), then precision = months, if match(YYYY-00-00T00:00:00Z) = years, etc. And only if there is no match, then assume a default precision of days, and raise the error if necessary (i.e. if the user really enters a wrong format or contradictory format and precision parameters).

That would probably require setting first the default value for precision to None, and assigning it to WikibaseDatePrecision.DAY only if necessary inside set_value. But maybe there is a reason not to do that. In any case, it otherwise seems redundant that every user will have to first check they are introducing the right precision for a given format that is not days.

@eloiferrer
Copy link
Contributor Author

Hi @LeMyst, I've implemented what I mean, but if this is too much of a change we can just revert the changes and merge your version.

@LeMyst LeMyst merged commit 842f03a into LeMyst:master Feb 6, 2023
@LeMyst
Copy link
Owner

LeMyst commented Feb 6, 2023

Thank you for this PR @eloiferrer ;)

@LeMyst LeMyst added the good first issue Good for newcomers label Feb 6, 2023
@eloiferrer eloiferrer deleted the fix_time_regex branch February 7, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants