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.

Sign up for GitHub

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

Jump to bottom

Use singledispatch in TimeSeriesFactory #5411

Merged
merged 4 commits into from
Jul 21, 2021
Merged

Use singledispatch in TimeSeriesFactory #5411

merged 4 commits into from
Jul 21, 2021

Conversation

dstansby
Copy link
Member

Fixes #2384. This is just refactoring, but makes it easier to see which bits of code handle the three input types of:

  • data, header, units tuple
  • file (including URL pointing to a file)
  • GenericTimeseries

@dstansby dstansby requested a review from a team as a code owner June 19, 2021 21:13
@nabobalis nabobalis added timeseries Affects the timeseries submodule Refactoring Code changes without affecting API (generally) labels Jun 19, 2021
@nabobalis
Copy link
Contributor

I feel like this is worth a trivial changelog entry.

@dstansby dstansby force-pushed the url-parse branch 2 times, most recently from 0c386e7 to 19c0e2e Compare June 23, 2021 16:03
@nabobalis nabobalis added the Needs Review Needs reviews before merge. label Jun 23, 2021
hayesla
hayesla approved these changes Jun 23, 2021
View reviewed changes
sunpy/timeseries/timeseries_factory.py Outdated Show resolved Hide resolved
wtbarnes
wtbarnes requested changes Jun 23, 2021
View reviewed changes
Copy link
Member

@wtbarnes wtbarnes left a comment

Choose a reason for hiding this comment

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

Needs a changelog and I think my suggested fix should fix the tests/doc builds

sunpy/timeseries/timeseries_factory.py Outdated Show resolved Hide resolved
@wtbarnes
Copy link
Member

Looks good to me (provided the tests pass!)

@nabobalis nabobalis merged commit 422cef2 into sunpy:main Jul 21, 2021
@dstansby dstansby deleted the url-parse branch July 21, 2021 16:07
@dstansby dstansby removed the Needs Review Needs reviews before merge. label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabobalis nabobalis nabobalis left review comments

@wtbarnes wtbarnes wtbarnes approved these changes

@hayesla hayesla hayesla approved these changes

Assignees
No one assigned
Labels
Refactoring Code changes without affecting API (generally) timeseries Affects the timeseries submodule
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Post Python 2: Refactor Map and Timeseries factory to use singledispatch in __call__
4 participants
@dstansby @nabobalis @wtbarnes @hayesla