Page MenuHomePhabricator

archivebot.str2time does not respect minutes and months
Open, NormalPublic

Description

minutes and months qualifiers where introduced for archivebot.str2localized_duration but is missing for str2time function. This causes a MalformConfigError:

>>> import pwb, pywikibot as py
>>> from scripts.archivebot import str2time as s2t
>>> s2t('300')
datetime.timedelta(0, 300)
>>> s2t('5m')

Traceback (most recent call last):
  File "<pyshell#62>", line 1, in <module>
    s2t('5m')
  File "scripts\archivebot.py", line 201, in str2time
    raise MalformedConfigError('Unrecognized parameter in template: %s' % string)
MalformedConfigError: Unrecognized parameter in template: 5m

Event Timeline

Xqt created this task.Oct 9 2016, 5:22 AM
Restricted Application added subscribers: pywikibot-bugs-list, Aklapper. · View Herald TranscriptOct 9 2016, 5:22 AM
Xqt triaged this task as Normal priority.Oct 9 2016, 5:23 AM

Change 314847 had a related patch set uploaded (by Xqt):
[bugfix] archivebot.str2time must respect 'm' and 'M' qualifiers

https://gerrit.wikimedia.org/r/314847

whym added a subscriber: whym.Nov 27 2016, 2:59 AM

Isn't it confusing to have 'm' and 'M' for different meanings? Do we have requests from other users to implement 'M' (months)?

Xqt added a comment.Nov 28 2016, 10:49 AM

We also have different meaning for 'm' and 'M' e.g. in strftime for month and minute. I think it is more usefull to have a month time identifier than seconds (which we already have) or maybe minutes. I guess it is minor confusing to have both, month ad minutes than having only one of it and you have to assumes what it means. N.B. the usage is documented.

whym added a comment.Dec 2 2016, 1:58 PM

From https://gerrit.wikimedia.org/r/#/c/314847/:

Whym
Nov 28 8:22 PM
Patch Set 3: Code-Review-1
but I'm not yet convinced that we need to introduce the 'M' qualifier. See https://phabricator.wikimedia.org/T147739#2825046 for discussion.

Xqt
Nov 28 10:30 PM
Patch Set 3:
the month or minute one? or both?

Whym
9:38 AM
Patch Set 3:
I meant to suggest dropping 'M' for month while keeping 'm' for minute.

Xqt
2:34 PM
Patch Set 3:
I disagree because months does make much more sense for archiving than minutes but I would keep both.

I thought that 'minute' had been around much longer than 'months', but it looks like I was wrong. As it looks like it was as recent as within a couple of months (https://gerrit.wikimedia.org/r/#/c/313352/), I would suggest cancelling the addition of both.

In response to:

I think it is more usefull to have a month time identifier than seconds (which we already have) or maybe minutes. I guess it is minor confusing to have both, month ad minutes than having only one of it and you have to assumes what it means.

I'd rather stick with 'd' for 'days' and 'h' for 'hours', which are by far the most commonly used ones. I'm ok with either keeping or abandoning 'y' and 'w' - rarely used but there is no harm. I think both 'M' for months and 'm' for 'minutes' should go, because the risk of confusing people is high while the benefit is low. You can easily use the non-ambiguous 'd' or 'w' instead.

As some evidence for the user's confusion, there are many instances of 'm' like "algo = old(2m)" and "algo = old(1m)" where users presumably used 'm' for 'months', not 'minutes':

We might have to a bit more carefully consider how/when to fade out 's' for 'seconds' (which is why I don't propose removing it yet). But in the case of 'M' and 'm', it is quite new and I think we should cancel the addition before it's too late to do so.

whym removed a subscriber: whym.Dec 14 2016, 5:59 AM
whym added a subscriber: whym.Jan 8 2017, 4:53 AM

Change 314847 merged by jenkins-bot:
archivebot stops supporting 'm' and 'M' qualifiers

https://gerrit.wikimedia.org/r/314847

Is this solved?

whym added a comment.May 28 2018, 11:14 AM

I think so, unless there are still plans/needs for supporting minute and month.

Xqt added a comment.May 28 2018, 11:44 AM

I think months should supported rather than seconds.

D3r1ck01 moved this task from Backlog to Needs Review on the Pywikibot board.Nov 5 2018, 11:29 AM
whym added a comment.Dec 19 2018, 9:39 AM

If the two time scales are still needed, I would suggest using longer forms ('600min' and '2mon'') or full spells ('600minute' and '2month', not sure if it should be plural or we should support both). The only problem for me was that it was confusing to have to distinguish minutes and months using lowercase 'm' and uppercase 'M'.

'min' and 'mon' might create other confusion in some non-English languages, though.