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

datetime to astropy Time: examples #2679

Merged
merged 7 commits into from Jul 14, 2018

Conversation

Projects
None yet
6 participants
@vn-ki
Copy link
Member

commented Jul 4, 2018

Todo:

  • Check that fermi 2 second difference is not a bug.
  • Make all CI pass.
@sunpy-bot

This comment has been minimized.

Copy link

commented Jul 4, 2018

Thanks for the pull request @vn-ki! Everything looks great!

@Cadair Cadair modified the milestones: 0.9.1, Time Jul 4, 2018

plt.axvline(parse_time(flares_hek[0].get('event_peaktime')))
plt.axvspan(parse_time(flares_hek[0].get('event_starttime')),
parse_time(flares_hek[0].get('event_endtime')),
plt.axvline(parse_time(flares_hek[0].get('event_peaktime')).datetime)

This comment has been minimized.

Copy link
@Cadair

Cadair Jul 4, 2018

Member

This is unfortunate. I wonder if we could implement a matplotlib converter to automatically support Time. @dstansby does that sound like a thing that could be done?

This comment has been minimized.

Copy link
@dstansby

dstansby Jul 4, 2018

Contributor

Hahaha, welcome to trying to use units with axvline! Is there a Time units converter for normal plotting already in astropy? If so this should work on certain minimum versions of astropy/matplotlib since I put some fixes in both recently.

This comment has been minimized.

Copy link
@vn-ki

vn-ki Jul 4, 2018

Author Member

I think we can use Time.plot_date here instead of .datetime. As mentioned in this comment, it should be more efficient.

This comment has been minimized.

Copy link
@Cadair

Cadair Jul 6, 2018

Member

nice

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

The figures failed and it said it uploaded the results but its not spat out a link.

@dstansby

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Probably need a rebase for the PR to pick up the new circleCI config

@pep8speaks

This comment has been minimized.

Copy link

commented Jul 4, 2018

Hello @vn-ki! Thanks for updating the PR.

Line 36:1: E265 block comment should start with '# '
Line 38:1: E265 block comment should start with '# '
Line 61:5: E265 block comment should start with '# '
Line 67:5: E265 block comment should start with '# '
Line 73:5: E265 block comment should start with '# '
Line 79:5: E265 block comment should start with '# '
Line 85:5: E265 block comment should start with '# '
Line 91:5: E265 block comment should start with '# '
Line 97:5: E265 block comment should start with '# '
Line 97:101: E501 line too long (101 > 100 characters)
Line 103:5: E265 block comment should start with '# '
Line 103:101: E501 line too long (108 > 100 characters)
Line 129:1: E265 block comment should start with '# '
Line 131:1: E265 block comment should start with '# '
Line 142:11: E225 missing whitespace around operator
Line 157:1: E265 block comment should start with '# '
Line 159:1: E265 block comment should start with '# '
Line 208:1: E265 block comment should start with '# '
Line 210:1: E265 block comment should start with '# '
Line 232:101: E501 line too long (106 > 100 characters)
Line 233:101: E501 line too long (121 > 100 characters)
Line 240:101: E501 line too long (138 > 100 characters)
Line 241:101: E501 line too long (132 > 100 characters)
Line 252:101: E501 line too long (131 > 100 characters)
Line 266:101: E501 line too long (123 > 100 characters)
Line 269:1: E265 block comment should start with '# '
Line 271:1: E265 block comment should start with '# '
Line 409:101: E501 line too long (108 > 100 characters)
Line 411:1: E265 block comment should start with '# '
Line 413:1: E265 block comment should start with '# '
Line 437:1: E265 block comment should start with '# '
Line 439:1: E265 block comment should start with '# '
Line 471:101: E501 line too long (107 > 100 characters)
Line 472:5: E265 block comment should start with '# '
Line 472:101: E501 line too long (103 > 100 characters)
Line 473:5: E265 block comment should start with '# '
Line 531:1: E265 block comment should start with '# '
Line 533:1: E265 block comment should start with '# '
Line 589:1: E265 block comment should start with '# '
Line 591:1: E265 block comment should start with '# '
Line 614:1: E265 block comment should start with '# '
Line 616:1: E265 block comment should start with '# '
Line 665:1: E265 block comment should start with '# '
Line 667:1: E265 block comment should start with '# '
Line 733:1: E265 block comment should start with '# '
Line 735:1: E265 block comment should start with '# '
Line 758:1: E265 block comment should start with '# '
Line 760:1: E265 block comment should start with '# '
Line 763:1: E265 block comment should start with '# '
Line 763:101: E501 line too long (102 > 100 characters)
Line 764:1: E265 block comment should start with '# '
Line 764:101: E501 line too long (105 > 100 characters)
Line 765:1: E265 block comment should start with '# '

Comment last updated on July 13, 2018 at 11:53 Hours UTC

@vn-ki vn-ki force-pushed the vn-ki:docs branch from 37a2d12 to 20e536c Jul 4, 2018

@vn-ki vn-ki force-pushed the vn-ki:docs branch from 8806638 to 14ac139 Jul 7, 2018

@vn-ki vn-ki force-pushed the vn-ki:docs branch from 6e39780 to 9af3223 Jul 11, 2018



def test_met_to_utc():
time = fermi.met_to_utc(500000000)

This comment has been minimized.

Copy link
@Cadair

Cadair Jul 11, 2018

Member

@aringlis @wafels We discovered by doing this that the result when converting Fermi MET to UTC is 2s different when using astropy Time than the previous datetime implementation. We talked through this and we think that it's because the previous implementation was not taking into account the two UTC leap seconds since the start of Fermi MET. Do either of you have any ideas if we could validate this somehow?

This comment has been minimized.

Copy link
@aringlis

aringlis Aug 1, 2018

Member

@Cadair hmm, interesting. A quick test is trying some different input time values, to see if the same 2s discrepancy is always there, e.g. try timestamps before and after the leap second.

I can also look into what the Fermi team itself does with their MET values.

This comment has been minimized.

Copy link
@aringlis

aringlis Aug 2, 2018

Member

See Issue #2710

@nabobalis nabobalis merged commit 1b5fd15 into sunpy:time Jul 14, 2018

10 checks passed

ci/circleci: astropy-time Your tests passed on CircleCI!
Details
ci/circleci: astropy-time-online Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 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
codecov/patch 100% of diff hit (target 84.04%)
Details
codecov/project 84.05% (+<.01%) compared to 5143953
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
sunpy-bot All checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.