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

DOC: improve removal for julian dates [ci doc] #26887

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Sep 23, 2023

The removal notice could state the conversions since they are one-liners. #26852

PR summary

PR checklist

@jklymak jklymak marked this pull request as ready for review September 23, 2023 02:08
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I find the two last sentences quite unsettling:

where some care needs to be taken with the accuracy of the timedelta64. Given the various
definitions of Julian date, users should exercise caution using these values.

If the replacement was just a simple „use this instead“, I‘d be all for it. But with the additions I feel not confident that I could use it. I‘m inclined to say, let‘s not bother with explaining a replacement, if it‘s not straight forward. I don‘t think many users have used Julian dates after all.

@jklymak
Copy link
Member Author

jklymak commented Sep 24, 2023

I think we should say something to allow users to calculate this themselves, though I guess they could look at the source code. I've double checked that our definition matches with Wikipedia's, and link them. I've also changed it to "Matplotlib was using EPOCH=..." to make it clear that is a choice, and link out to Wikipedia if people need it.

(date - np.datetime64(EPOCH)) / np.timedelta64(1, 'D')``. Conversely, a Julian
date was converted to datetime as ``date = np.timedelta64(int(jdate * 24 *
3600), 's') + np.datetime64(EPOCH)``. Matplotlib was using
``EPOCH='-4713-11-24T12:00'`` so that 2000-01-01 at 12:00 is 2_451_545.0 (see
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
``EPOCH='-4713-11-24T12:00'`` so that 2000-01-01 at 12:00 is 2_451_545.0 (see
``EPOCH='-4713-11-24T12:00'`` so that 2000-01-01 at 12:00 is ``2_451_545.0`` (see

possibly to clarify that it is a "Python float".

But clearly an improvement to me.

@timhoffm timhoffm added this to the v3.9.0 milestone Sep 27, 2023
@timhoffm timhoffm merged commit 11aa656 into matplotlib:main Sep 27, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants