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

Migrate to Astropy Time #2691

Merged
merged 218 commits into from Dec 14, 2018

Conversation

@Cadair
Member

Cadair commented Jul 18, 2018

This is the port of SunPy to Astropy time.

This currently has #2666 in it.

Todo:

  • Changelogs
  • Reset CI to match master

This PR closes #993 closes #994 closes #2155

vn-ki and others added some commits May 1, 2018

parse_time changes
date, datetime, astropy Time, pandas Series, pandas DatetimeIndex, pandas Timestamp
Make requested changes
Change __init__.py
Include __all__
Changes to test_time
@nabobalis

This comment has been minimized.

Contributor

nabobalis commented Nov 17, 2018

Ok, Im 99% sure I've addressed all the comments that were missed the first time and assuming I did not screw something up just now. The tests should all pass.

Should be all sorted now!

@nabobalis

LETS DO THIS

Show resolved Hide resolved sunpy/time/time.py Outdated
@Cadair

I have reviewed the docs and think we could tidy a little more. Probably worth doing now but if we don't have time before merge then we can do it later.

Show resolved Hide resolved docs/guide/time.rst
Show resolved Hide resolved docs/guide/time.rst Outdated
Show resolved Hide resolved docs/guide/time.rst Outdated
Show resolved Hide resolved sunpy/time/time.py Outdated
Show resolved Hide resolved sunpy/time/time.py Outdated
numpy.datetime64, pandas.Timestamp ]
Time to parse.
format : [ 'jd', 'mjd', 'decimalyear', 'unix', 'cxcsec', 'gps',

This comment has been minimized.

@Cadair

Cadair Nov 20, 2018

Member

These are not types they are values, so this line should read format : `str` and the options should be enumerated below. See the fact this dosen't render well currently: https://4707-2165383-gh.circle-artifacts.com/0/root/project/docs/_build/html/api/sunpy.time.parse_time.html#sunpy.time.parse_time

This comment has been minimized.

@Cadair

Cadair Nov 20, 2018

Member

In fact the way astropy lists the allowed formats is quite clever: http://docs.astropy.org/en/stable/api/astropy.time.Time.html#astropy.time.Time we should do that.

This comment has been minimized.

@Cadair

Cadair Nov 20, 2018

Member

It's also worth noting that in the numpy doc standard it says the following:

When a parameter can only assume one of a fixed set of values, those values can be listed in braces, with the default appearing first:

order : {'C', 'F', 'A'}
    Description of `order`.

so we could use our format docstring decorator to auto fill the list of formats.

Show resolved Hide resolved docs/code_ref/time.rst Outdated
@Cadair

Some more comments 😀

The most commonly used are strings and we support a selection of formats
which are matched using regrex. We currently support the following style of string formats::

"2007-05-04T21:08:12.999999"

This comment has been minimized.

@Cadair

Cadair Nov 20, 2018

Member

I like having this list, I do wonder if it would be possible to autogenerate it though?

This comment has been minimized.

@nabobalis

nabobalis Nov 20, 2018

Contributor

Well I wanted to add the regex and time format variables in the docs but I couldn't work out a way. So I just did this for now.

Show resolved Hide resolved docs/guide/time.rst Outdated
Show resolved Hide resolved docs/guide/time.rst Outdated
>>> parse_time(pandas.DatetimeIndex(time_ranges)) # pandas.DatetimeIndex
<Time object: scale='utc' format='datetime' value=[datetime.datetime(2007, 5, 1, 0, 0) datetime.datetime(2007, 5, 2, 0, 0)]>

>>> import numpy as np

This comment has been minimized.

@Cadair

Cadair Nov 20, 2018

Member

I would put all the imports at the top.

This comment has been minimized.

@nabobalis

nabobalis Nov 20, 2018

Contributor

I wanted to seperate the examples out by module.

Show resolved Hide resolved docs/guide/time.rst Outdated
Show resolved Hide resolved sunpy/time/time.py Outdated
Show resolved Hide resolved sunpy/time/time.py Outdated

Cadair and others added some commits Nov 21, 2018

Merge pull request #2852 from Cadair/parse_time
Parse time docstring trickery
Remove astropy Time compatibility layer
This bumps us to an astropy 3.1 dep
Show resolved Hide resolved tox.ini Outdated
@nabobalis

This comment has been minimized.

Contributor

nabobalis commented Dec 14, 2018

So the only test issues are database.rst.

@nabobalis

This comment has been minimized.

Contributor

nabobalis commented Dec 14, 2018

Travis isuse same as Zeep's PR.

@Cadair

This comment has been minimized.

Member

Cadair commented Dec 14, 2018

I hereby declare this is close enough.

@Cadair Cadair merged commit bfc2828 into master Dec 14, 2018

10 of 12 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
ci/circleci: 32bit Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-37 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy #20181214.5 succeeded
Details

@Cadair Cadair deleted the time branch Dec 14, 2018

@Cadair Cadair moved this from Release Blocking Items to Finished in SunPy 1.0 Dec 14, 2018

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