BUG: Raise if not able to get offset from tz #16129
Conversation
| can u add a test? (e.g. pass in something like 'foo') also can add a windows timezone test (like your issue) but need to skip if it in windows and a whatsnew note - bug fix section |
pandas/_libs/tslib.pyx Outdated
| offset = tzinfo.utcoffset(obj) | ||
| if offset is None: | ||
| raise NotImplementedError("Handling of tzinfo '%s' without " | ||
| "utcoffset not implemented" % tzinfo) |
There was a problem hiding this comment.
can u use .format() for string formatting
There was a problem hiding this comment.
done - I'll add a test shortly
jreback added Error Reporting 
Codecov Report
@@ Coverage Diff @@ ## master #16129 +/- ## ========================================== - Coverage 90.86% 90.83% -0.03% ========================================== Files 159 159 Lines 50771 50771 ========================================== - Hits 46133 46119 -14 - Misses 4638 4652 +14
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@ ## master #16129 +/- ## ========================================== - Coverage 90.86% 90.83% -0.03% ========================================== Files 159 159 Lines 50771 50771 ========================================== - Hits 46133 46119 -14 - Misses 4638 4652 +14
Continue to review full report at Codecov.
|
| tm.assert_numpy_array_equal(result, np.array( | ||
| [tslib.iNaT], dtype=np.int64)) | ||
| | ||
| |
There was a problem hiding this comment.
do like this:
# add to import at the top from pandas.compat import is_platform_windows @pytest.mark.skipif(not is_platform_windows(), reason="windows only") def test_win32_tz_object(): ..... There was a problem hiding this comment.
use assert ut == expected (we are moving away from inherity from tm.TestCase entirely.
| def test_win32_tz(self): | ||
| tz = win32timezone.TimeZoneInfo('Eastern Standard Time') | ||
| wt = pywintypes.TimeType(2008, 5, 12, 9, 50, tzinfo=tz) | ||
| tt = Timestamp(wt) |
There was a problem hiding this comment.
shouldn't this now raise NotImplementedError?
There was a problem hiding this comment.
No, it now falls through and gets the timezone correctly because of the NotImplementedError. That's raised when trying to get the fixed offset. It does however log that it's ignoring that error, which given that it's now working makes me think that actually raising that exception isn't the best thing to do.
I'll close this PR for now and re-open once I've had a chance to look at the best way to get this to work without logging warnings, and taking your feedback re the test cases into consideration.
| Closing for now - will open a new one that handles the windows timezone objects correctly without logging warnings. |
| ok, just reopen this one. no need to create another |
2 participants
git diff upstream/master --name-only -- '*.py' | flake8 --diff