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

Use tz.gettz() instead of zoneinfo.gettz() #9123

Merged
merged 1 commit into from
May 15, 2015
Merged

Conversation

jlec
Copy link
Contributor

@jlec jlec commented Dec 21, 2014

zoneinfo.gettz() seems to have problems (1 & 2) on system which do not install
the zoninfo tarball (e.g. Debian, Gentoo and Fedora) but rely on the system
zoneinfo files. This results in test failures (3 & 4)
tz.gettz() doesn't suffer from this problem.

1 dateutil/dateutil#8
2 dateutil/dateutil#11
closes #9059
closes #8639
closes #10121

Signed-off-by: Justin Lecher jlec@gentoo.org

@jreback
Copy link
Contributor

jreback commented Dec 21, 2014

the is an api issue for dateutil

the current settings work on all platforms
except yours

pandas should not be fixing the problem rather dateutil should

@jreback
Copy link
Contributor

jreback commented Dec 22, 2014

@jlec so this completely breaks windows. That is the reason I changed it in the first place. That said if you wanted to put in different imports (e.g. use the current code if on windows, else use what you did). I think that would be acceptable, until dateutil actually fixes this.

lmk

@jreback jreback added Compat pandas objects compatability with Numpy or Python functions Windows Windows OS Timezones Timezone data dtype Testing pandas testing functions or related to the test suite labels Dec 22, 2014
@jreback jreback added this to the 0.16.0 milestone Dec 22, 2014
@jreback
Copy link
Contributor

jreback commented Jan 18, 2015

@jlec any interest in doing the change I propose above?

@jlec
Copy link
Contributor Author

jlec commented Jan 19, 2015

@jreback I didn't saw your direct reply. I will look into this this week.

@jlec
Copy link
Contributor Author

jlec commented Jan 26, 2015

@jreback which way do you prefer to check for windows, os.name or sys.platform?

@jreback
Copy link
Contributor

jreback commented Jan 26, 2015

use sys.platform != 'win32'

@jlec
Copy link
Contributor Author

jlec commented Feb 23, 2015

Sorry for the long delay. Somehow I don't get mails from GH correctly

@@ -5021,7 +5021,10 @@ def test_getitem_setitem_datetime_tz_pytz(self):
def test_getitem_setitem_datetime_tz_dateutil(self):
tm._skip_if_no_dateutil();
from dateutil.tz import tzutc
from dateutil.zoneinfo import gettz
if sys.platform != 'win32':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe make a function in pandas/compat/__init__.py like: is_platform_windows()/mac/linux just for convenience

@jreback jreback modified the milestones: 0.16.0, Next Major Release, 0.16.1 Mar 3, 2015
@jreback jreback modified the milestones: 0.17.0, 0.16.1 Apr 21, 2015
@jreback
Copy link
Contributor

jreback commented May 9, 2015

@jlec can you update?

@jreback
Copy link
Contributor

jreback commented May 10, 2015

@jlec you need to rebase on master, see howtos in here

@jlec
Copy link
Contributor Author

jlec commented May 12, 2015

It's not the wrong rebase, I have a merge commit. I will try to clean that up.

@jreback
Copy link
Contributor

jreback commented May 15, 2015

ok, pls add a release note in v0.17.0.txt mentioned all the above issues (you can just list them with a single note). and squash to a single commit.

@jlec jlec force-pushed the tz branch 2 times, most recently from d638212 to 6dedd02 Compare May 15, 2015 15:46
python-dateutil provides two implementations for gettz(), tz.gettz() and
zoneinfo.gettz(). The former tries first to use system provided timezone data,
where as the later always uses a bundled tarball. Upstreams recommandation
for library consumers is only using tz.gettz() (1 & 2). Further more, on
system which do not install the zoninfo tarball (e.g. Debian, Gentoo and
Fedora) but rely on the system zoneinfo files the direct usage of
zoneinfo.gettz() creates problems which result in test failures (3 - 6).

For compatibility in pandas code

    pandas.tslib._dateutil_gettz()

should be used.

1 dateutil/dateutil#8
2 dateutil/dateutil#11
3 pandas-dev#9059
4 pandas-dev#8639
5 pandas-dev#10121
6 pandas-dev#9663

Signed-off-by: Justin Lecher <jlec@gentoo.org>
@jlec
Copy link
Contributor Author

jlec commented May 15, 2015

Hopefully everything is now respecting the contribution guidelines. If not, please tell me what needs to be fixed.

@jreback
Copy link
Contributor

jreback commented May 15, 2015

ok, ping me when travis is green and I will merge.

@jreback jreback merged commit 48bc59b into pandas-dev:master May 15, 2015
@jreback
Copy link
Contributor

jreback commented May 15, 2015

@jlec thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compat pandas objects compatability with Numpy or Python functions Testing pandas testing functions or related to the test suite Timezones Timezone data dtype Windows Windows OS
Projects
None yet
3 participants