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

ArrowParseWarning, for the .get() method #296

Closed
ryanmjacobs opened this issue Aug 1, 2019 · 6 comments
Closed

ArrowParseWarning, for the .get() method #296

ryanmjacobs opened this issue Aug 1, 2019 · 6 comments

Comments

@ryanmjacobs
Copy link

After updating my system today, watson sync triggers this warning message a couple of times:

/usr/lib/python3.7/site-packages/arrow/factory.py:201: ArrowParseWarning:

The .get() parsing method without a format string will parse more strictly in
version 0.15.0. See https://github.com/crsmithdev/arrow/issues/612 for more details.

I grepped the source code for all calls to arrow.get(),

ryan@mu2 /tmp/Watson $ ag -Q 'arrow.get('
watson/watson.py
107:            date = arrow.get(date)
229:            self._last_sync = arrow.get(0)

watson/utils.py
377:    datetime_from = arrow.get(
380:    datetime_to = arrow.get(

watson/fullmoon.py
233:    return arrow.get(last)

watson/frames.py
14:                start = arrow.get(start)
17:                stop = arrow.get(stop)
28:            updated_at = arrow.get(updated_at)

watson/cli.py
77:                date = arrow.get(value)
116:        return arrow.get(cur_time).replace(tzinfo=local_tz)
633:                arrow.get(report['timespan']['from'])
643:                arrow.get(report['timespan']['from'])
646:                arrow.get(report['timespan']['to'])
1235:            start = arrow.get(data['start'], datetime_format).replace(
1237:            stop = arrow.get(data['stop'], datetime_format).replace(

tests/test_watson.py
47:    assert watson.current['start'] == arrow.get(4000)
91:    now = arrow.get(4123)
101:    assert watson.last_sync == arrow.get(0)
106:    assert watson.last_sync == arrow.get(0)
132:    assert watson.last_sync == arrow.get(0)
143:    assert watson.frames[0].start == arrow.get(4000)
144:    assert watson.frames[0].stop == arrow.get(4010)
154:    assert watson.frames[0].start == arrow.get(4000)
155:    assert watson.frames[0].stop == arrow.get(4010)
322:        time_obj = arrow.get(time_str)
327:        time_obj = arrow.get(time_str)

tests/test_utils.py
76:        original_start = arrow.get(monday_start, "YYYY MM D")
77:        result = arrow.get(new_start, "YYYY MM D")

I'm thinking that all we have to do is change every call to use the date format YYYY MM D. But I'm not super familiar with the codebase, so I don't know if any other date formats were used. I doubt it, but you can never be too careful.

@systemcatch
Copy link

Hey @ryanmjacobs, I'm one of the maintainers of arrow. These warnings are a result of long standing str parsing bugs that we're attempting to fix in Version 0.15.0.

Looking through your grep the potential breakages will be in;

watson/utils.py
377:    datetime_from = arrow.get(
380:    datetime_to = arrow.get(

watson/frames.py
14:                start = arrow.get(start)
17:                stop = arrow.get(stop)
28:            updated_at = arrow.get(updated_at)

watson/cli.py
77:                date = arrow.get(value)
116:        return arrow.get(cur_time).replace(tzinfo=local_tz)
633:                arrow.get(report['timespan']['from'])
643:                arrow.get(report['timespan']['from'])
646:                arrow.get(report['timespan']['to'])

Whether something breaks will depend on exactly what format is being parsed. I think utils.py is safe from reading the code, anything like YYYY-MM-DD HH:mm:ss should be also fine.

Version 0.15.0 is on track for being released at the end of August, let me know if you need anymore info.

ref arrow-py/arrow#612

@ryanmjacobs
Copy link
Author

Hey @systemcatch, thanks for the speedy follow-up. That's super helpful.

I went ahead and started doing replacements on watson/utils.py, watson/frames.py, and watson/cli.py.


Anyways, here's my progress so far.

I've been cautious when adding the format strings to .get() calls — because I'm scared of assuming the wrong format. So what I'm doing, is I'm naively printing the datetime_string before I feed it into .get(). That way I can verify my format assumption. (But even still, I'm not 100% sure that what I see on my system in this one testing instance, will apply on everyone else's system and mine across various use cases. Localetime may or may not play a role. I don't want to push any fixes that I'm not 100% sure about it's effect.)

There are some inconsistencies regarding which format of datetime strings are used across the codebase. I'd love if the author/maintainer could pitch in and describe confidently: which formats are used and where.

  • In cli.py line 78, this one pops up 2019-07-26T14:44:03.158743-04:00.
  • In frames.py near line 14, start and stop appear to be Unix epochs. I'm getting integer values such as 1563547439. These work fine .get() without any modification.

@systemcatch
Copy link

* In `cli.py` line 78, this one pops up `2019-07-26T14:44:03.158743-04:00`.

That format is fine to use.

* In `frames.py` near line 14, `start` and `stop` appear to be Unix epochs. I'm getting integer values such as `1563547439`. These work fine `.get()` without any modification.

As long as the timestamps are ints/floats and not strings it's all good.

@ybc37
Copy link
Contributor

ybc37 commented Aug 30, 2019

Hey!

I agree with @ryanmjacobs. It would be great if someone who is more familiar with the code base could help here.

Meanwhile, if someone is annoyed of all the warnings, it's possible to suppress those: master...nesmyslny:ignore-arrow-parse-warnings (documented here: arrow-py/arrow#612 (comment))
Or just alias watson to watson 2>/dev/null or so.

Obviously that are nasty workarounds 😉 Just temporary relief 😄

@systemcatch
Copy link

Just a quick note that we released arrow 0.14.7 a few days ago. The ArrowParseWarnings have been dialed back and will just behave like normal deprecation warnings (no suppression needed). There's no other changes in that version so if you want the warnings gone it should be a safe upgrade. 🤞

@ryanmjacobs
Copy link
Author

See #313 for relevant PR. I'm closing this issue now.

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

No branches or pull requests

4 participants