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

allow back-compat of timezone specifier #7074

Closed
jreback opened this issue Jan 20, 2016 · 19 comments
Closed

allow back-compat of timezone specifier #7074

jreback opened this issue Jan 20, 2016 · 19 comments

Comments

@jreback
Copy link

jreback commented Jan 20, 2016

recently merged PR on fixing datetime w/tz parsing: #6453

we are hitting this in lots of places in here

I can simply put in a compat function to make this work with many versions of numpy (and not show the warning), but

it might make sense for back-compat to allow acceptance:

np.datetime64('2007-01-01 09:00:00.001Z') w/o showing the deprecation warning as this is de-facto (at present) a naive timezone in numpy anyhow AND this allows the same code to work in numpy < 1.11

(of course if a non-UTC tz, e.g. anything beyond the Z) then I would continue to do as the revised function.

@jreback
Copy link
Author

jreback commented Jan 20, 2016

cc @shoyer

@charris charris added this to the 1.11.0 release milestone Jan 20, 2016
@charris
Copy link
Member

charris commented Jan 23, 2016

@shoyer This looks like the last thing needed for the 1.11.0 beta.

@charris
Copy link
Member

charris commented Jan 23, 2016

@jreback Is your main concern a flood of deprecation warnings?

@jreback
Copy link
Author

jreback commented Jan 23, 2016

well you need different code in numpy < 1.11 from >= 1.11 to get equivalent results.

@charris
Copy link
Member

charris commented Jan 23, 2016

So, perhaps a FutureWarning and a long wait?

@charris
Copy link
Member

charris commented Jan 23, 2016

@jreback What would be the best fix from your point of view?

@jreback
Copy link
Author

jreback commented Jan 23, 2016

I @shoyer had the right soln. maybe leave the change along and if it is a UTC zone (e.g. a single 'Z') then show the FutureWarning but accept it. That way the user is notified but doesn't have to change it immediately (so its back compat).

@charris
Copy link
Member

charris commented Jan 23, 2016

I think we need the back compat. What numpy versions do you officially support?

@jreback
Copy link
Author

jreback commented Jan 23, 2016

1.7+

@njsmith
Copy link
Member

njsmith commented Jan 23, 2016

maybe leave the change along and if it is a UTC zone (e.g. a single 'Z') then show the FutureWarning but accept it. That way the user is notified but doesn't have to change it immediately (so its back compat).

That's exactly what current master & the 1.11 branch are doing, right? except that they use DeprecationWarning instead of FutureWarning? This is what deprecation warnings are for, to give a period where the old behavior continues to be accepted for back-compat :-)

@jreback
Copy link
Author

jreback commented Jan 23, 2016

@njsmith BUT the problem is that I cannot change it AND have compat with previous versions. So I have either to have to accept a warning (and catch it only in >= 1.11 and not change any code), or use a compat function so that I don't get a warning in prior versions.

I cannot simply change it if I want to maintain across numpy versions.

@charris
Copy link
Member

charris commented Jan 23, 2016

Maybe PendingDeprecationWarning ;) Seeing how old 1.7 is, it looks like it will be a while... We do like to have backward compatible solutions available for deprecated features.

@njsmith
Copy link
Member

njsmith commented Jan 24, 2016

@jreback: Still not sure I understand :-/. I think your options are:

  1. Just ignore the warning and keep doing what you're doing; after a while bump your minimum required numpy version to 1.11 and switch to naive time strings.
  2. Check the numpy version, and give old numpy the Zulu time strings and new numpy the naive time strings; eventually drop the old numpy compat code.

I don't really see what numpy can do that would give you any other options, other than (3) just don't make the change at all. But I think we do want to eventually get rid of allowing the timezone in the string for naive datetime64s -- if nothing else it's necessary to clear the way to having real timezone support later!

I can certainly see an argument that we should give the handling of Z timezones a longer deprecation period than we allow for other timezones, given that pandas uses them. Beyond that, what concretely do you want numpy to do?

@jreback
Copy link
Author

jreback commented Jan 24, 2016

guess I will have to do option 2.

@charris
Copy link
Member

charris commented Jan 24, 2016

@jreback If you make a shim function, could you post it here? That way folks running into the warning might find it. Also, could the DeprecationWarning message be improved?

@jreback
Copy link
Author

jreback commented Jan 24, 2016

pandas-dev/pandas#12127

your messaging is good.

def tz_replacer(s):
    if isinstance(s, string_types):
        if s.endswith('Z'):
            s = s[:-1]
        elif s.endswith('-0000'):
            s = s[:-5]
    return s


def np_datetime64_compat(s, *args, **kwargs):
    """
    provide compat for construction of strings to numpy datetime64's with
    tz-changes in 1.11 that make '2015-01-01 09:00:00Z' show a deprecation
    warning, when need to pass '2015-01-01 09:00:00'
    """

    if not _np_version_under1p11:
        s = tz_replacer(s)
    return np.datetime64(s, *args, **kwargs)


def np_array_datetime64_compat(arr, *args, **kwargs):
    """
    provide compat for construction of an array of strings to a
    np.array(..., dtype=np.datetime64(..))
    tz-changes in 1.11 that make '2015-01-01 09:00:00Z' show a deprecation
    warning, when need to pass '2015-01-01 09:00:00'
    """

    if not _np_version_under1p11:

        # is_list_like
        if hasattr(arr, '__iter__') and not \
           isinstance(arr, string_and_binary_types):
            arr = [tz_replacer(s) for s in arr]
        else:
            arr = tz_replacer(s)

    return np.array(arr, *args, **kwargs)

@jakirkham
Copy link
Contributor

Sorry to be pushy, but this is the last thing on the 1.11.0 release milestone. Is there something that is actionable here? I am willing to help if there is some clear actionable. Alternatively, could we make an alpha release while anything else here gets resolved and then add it in a beta?

@jreback
Copy link
Author

jreback commented Jan 25, 2016

I guess nothing to do

@charris
Copy link
Member

charris commented Jan 25, 2016

Closing this then.

@charris charris closed this as completed Jan 25, 2016
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