4
\$\begingroup\$

The following code is intended to parse a string of the following format and return whether the current time falls in that window:

start_time-stop_time [(day|day-day[, ...])][; ...]

For example,

0900-1200 (mon-wed, sat); 0100-0200 (sun, tue)

The code for parsing that type of string is here:

days = ['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun']
#grammar definition
hour = pp.Regex('[01]\d|2[0-3]')
minute = pp.Regex('[0-5]\d')
miltime = hour.setResultsName('hour') + minute.setResultsName('minute')
dayrange = pp.oneOf(days) + pp.Suppress('-') + pp.oneOf(days)                            #day-day
dayspec = '(' + pp.delimitedList(pp.Group(dayrange | pp.oneOf(days)), delim=',').setResultsName('dayspec') + ')'
window = miltime.setResultsName('start') + '-' + miltime.setResultsName('end') + pp.Optional(dayspec)
spec = pp.StringStart() + pp.delimitedList( pp.Group(window), delim=';' ) + pp.StringEnd()

#parse and act
#return True instead of return condition so that when it fails, it tries the next window
uptime_spec = uptime_spec.strip().lower()
for window in spec.parseString(uptime_spec):
  start = time(int(window.start.hour), int(window.start.minute))
  end = time(int(window.end.hour), int(window.end.minute))
  if start > end:
    raise pp.ParseBaseException('Start time after end time')
  if start < datetime.now().time() < end:
    if not window.dayspec:                #if there's no dayspec, any day is fine
      return True
    else:
      for day in window.dayspec:
        if len(day.asList()) == 1:        #a single day
          if datetime.now().strftime('%a').lower() == day[0]:
            return True
        else:                             #a range
          start = days.index(day[0])
          end = days.index(day[1])
          if start <= end:                #the range doesn't wrap
            if start <= datetime.now().weekday() <= end:
              return True
          else:                           #the range wraps, check that today isn't outside the range
            if not (end < datetime.now().weekday() < start):
              return True

return False

While this solution works, the parsing bit seems rather ugly, especially how I deal with days and day ranges. That part would be easier if I could just test if today's day of the week was in an allowed set of days, and I feel like that might be possible using setParseAction, but I have no idea how to do it.

Any other suggestions? I'm rather new at this parsing thing.

\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

1. Bugs

  1. Supposing that I want to specify an interval of time spanning midnight, for example starting at 23:00 on Monday and ending at 01:00 on Tuesday, how do I do it? The best I can do seems to be:

    2300-2359 (mon); 0000-0100 (tue)

    but this leaves the minute between 23:59 and midnight on Monday uncovered! One way to fix this would be to allow the input "2400" to represent the midnight at the end of the day.

  2. datetime.now() is called several times during the validation. This could lead to an incorrect result if the code is called at the wrong moment. For example, suppose that my specification is:

    1200-1300 (mon); 1100-1200 (mon)

    and the function is called just before 12:00 on Monday. When the first window is checked, datetime.now() returns 11:59:59.999, and so the window does not match. But when the second window is checked, datetime.now() returns 12:00:00.001 and so the second window does not match either! But these two windows ought to cover the whole interval from 11:00 to 13:00 on Monday.

    If you were to fix the midnight problem in §1.1 above, then there would be a similar race condition in the day handling logic.

    So it is necessary to call datetime.now() exactly once and remember the result. Or better still, change the function so that it takes the time to be tested as a parameter.

  3. If the day specification consists of a single day then the code tests for a matching day like this:

    datetime.now().strftime('%a').lower() == day[0]

    However, strftime is affected by the current locale:

    >>> import datetime, locale
>>> locale.setlocale(locale.LC_TIME, 'fr_FR')
>>> datetime.datetime.now().strftime('%a')
'Sam'

    so this will go wrong if the locale is not English. It would be more reliable to use:

    now.weekday() == days.index(day)

2. Review

  1. The Python style guide (PEP8) recommends four spaces per indentation level, and a maximum of 79 characters per line. You're not obliged to follow this guide, but it'll make it easier to collaborate with other Python programmers if you do. (For example, we wouldn't have to scroll the code horizontally to read it here at Code Review.)

  2. In the pp.delimitedList call, the keyword argument delim defaults to ',' so this can be omitted in the definition of dayspec.

  3. The ParserElement class has an abbreviated syntax for the setResultsName method:

    You can also set results names using the abbreviated syntax, expr("name") in place of expr.setResultsName("name")

    Using this would help make the grammar more readable.

  4. The correspondence between grammatical elements and names is inexact. It seems to me that it would make more sense for the hour element to be named 'hour', like this:

    hour = pp.Regex('[01]\d|2[0-3]')('hour')
minute = pp.Regex('[0-5]\d')('minute')
time = hour + minute

  5. Assigning a few more grammar elements would make things clearer and help to keep the code within 79 columns, for example:

    windows = pp.delimitedList(pp.Group(window), delim=';')
spec = pp.StringStart() + windows + pp.StringEnd()
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.