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

parser raises TypeError in wrapper logic #981

Closed
pganssle opened this issue Nov 19, 2019 · 4 comments · Fixed by #987
Closed

parser raises TypeError in wrapper logic #981

pganssle opened this issue Nov 19, 2019 · 4 comments · Fixed by #987

Comments

@pganssle
Copy link
Member

pganssle commented Nov 19, 2019

Apparently sometimes the parser catches a type of ValueError other than the ones we raise - specifically IllegalMonthError, which takes as an argument something other than a string. Minimal reproducer:

>>> from dateutil import parser
>>> parser.parse("0-100")
>>> dateutil.parser.parse("0-100")

This raises the following error:

---------------------------------------------------------------------------
IllegalMonthError                         Traceback (most recent call last)
/usr/lib/python3.7/site-packages/dateutil/parser/_parser.py in parse(self, timestr, default, ignoretz, tzinfos, **kwargs)
    654         try:
--> 655             ret = self._build_naive(res, default)
    656         except ValueError as e:

/usr/lib/python3.7/site-packages/dateutil/parser/_parser.py in _build_naive(self, res, default)
   1237
-> 1238             if cday > monthrange(cyear, cmonth)[1]:
   1239                 repl['day'] = monthrange(cyear, cmonth)[1]

/usr/lib/python3.7/calendar.py in monthrange(year, month)
    123     if not 1 <= month <= 12:
--> 124         raise IllegalMonthError(month)
    125     day1 = weekday(year, month, 1)

IllegalMonthError: bad month number 0; must be 1-12

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-3-f7db2d9dc78c> in <module>
----> 1 dateutil.parser.parse("0-100")

/usr/lib/python3.7/site-packages/dateutil/parser/_parser.py in parse(timestr, parserinfo, **kwargs)
   1372         return parser(parserinfo).parse(timestr, **kwargs)
   1373     else:
-> 1374         return DEFAULTPARSER.parse(timestr, **kwargs)
   1375
   1376

/usr/lib/python3.7/site-packages/dateutil/parser/_parser.py in parse(self, timestr, default, ignoretz, tzinfos, **kwargs)
    655             ret = self._build_naive(res, default)
    656         except ValueError as e:
--> 657             six.raise_from(ParserError(e.args[0] + ": %s", timestr), e)
    658
    659         if not ignoretz:

TypeError: unsupported operand type(s) for +: 'int' and 'str'

The core of the issue is this line:

        try:
            ret = self._build_naive(res, default)
        except ValueError as e:
            six.raise_from(ParserError(e.args[0] + ": %s", timestr), e)

Since e.args[0] is apparently occasionally an integer.

I guess we can solve this this way:

except ValueError as e:
    six.raise_from(ParseError(str(e) + ": %s", timestr), e)

This will need a regression test. This is apparently the cause of matplotlib/matplotlib#15726.

@mitar
Copy link

mitar commented Nov 25, 2019

We also had an issue with this after upgrade to 2.8.1: https://gitlab.com/datadrivendiscovery/common-primitives/issues/119

Our example:

dateutil.parser.parse('1986/87', default=datetime.datetime.fromtimestamp(0, datetime.timezone.utc), fuzzy=False)

@seitenbau-govdata

This comment has been minimized.

@pganssle
Copy link
Member Author

Please do not add "I'm affected by this" or "+1" comments here.

I've outlined the solution in the first post, I would be happy to review and merge a PR fixing it, but I have a large backlog.

@dateutil dateutil deleted a comment from ashwini-balnaves Dec 23, 2019
@dateutil dateutil deleted a comment from eastface Dec 23, 2019
eastface added a commit to eastface/dateutil that referenced this issue Dec 23, 2019
…ested.

Guard against an integer value in ValueError args.  This is known to
occur with IllegalMonthError.
pganssle pushed a commit to eastface/dateutil that referenced this issue Jan 2, 2020
In attempting to pass-through the string representation of an exception
we are wrapping, we made the erroneous assumption that `args[0]` would
always be a string (or something that can concatenate cleanly with a
string). This turns out not to be the case with `IllegalMonthError`,
where it is an integer, so to avoid raising an erroneous `TypeError`, we
first convert the wrapped exception to a string.

See GH issue dateutil#981.
pganssle pushed a commit to eastface/dateutil that referenced this issue Jan 2, 2020
In attempting to pass-through the string representation of an exception
we are wrapping, we made the erroneous assumption that `args[0]` would
always be a string (or something that can concatenate cleanly with a
string). This turns out not to be the case with `IllegalMonthError`,
where it is an integer, so to avoid raising an erroneous `TypeError`, we
first convert the wrapped exception to a string.

See GH issue dateutil#981.
@pganssle pganssle modified the milestones: Bugfix release, 2.8.2 Jan 2, 2020
@librehat
Copy link

@pganssle not trying to say +1 or something, but where can I find your solution in the "first post"?

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

Successfully merging a pull request may close this issue.

4 participants