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

update for upstream changes to get_data #20

Merged
merged 1 commit into from
May 25, 2023

Conversation

kecnry
Copy link
Member

@kecnry kecnry commented May 24, 2023

This PR will be needed if spacetelescope/jdaviz#2199 and javerbukh/jdaviz#15 spacetelescope/jdaviz#2214 are adopted upstream to support temporal subsets.

Comment on lines +100 to +155
subset : str, optional
Subset that is to be applied (as a mask) to the data before it is returned.
Copy link
Member Author

Choose a reason for hiding this comment

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

eventually we might want a separate spatial_subset that can be applied to TPFs for extraction of light curves... if/when that happens, we could consider renaming this to temporal_subset (or we could do it now if everyone thinks that would be more clear... I'm just worried that it could still be a bit confusing since they can be in either phase or time-space).

@kecnry kecnry requested a review from bmorris3 May 25, 2023 13:39
@kecnry kecnry force-pushed the get-data-updates branch from 5a31ae8 to a9e98b0 Compare May 25, 2023 18:22
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (844bde4) 83.54% compared to head (a9e98b0) 83.54%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #20   +/-   ##
=======================================
  Coverage   83.54%   83.54%           
=======================================
  Files          15       15           
  Lines         693      693           
=======================================
  Hits          579      579           
  Misses        114      114           
Impacted Files Coverage Δ
lcviz/helper.py 93.75% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@bmorris3 bmorris3 left a comment

Choose a reason for hiding this comment

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

This looks good, I think it's clear enough for now.

@kecnry kecnry merged commit bc42ae3 into spacetelescope:main May 25, 2023
@kecnry kecnry deleted the get-data-updates branch May 25, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants