-
Notifications
You must be signed in to change notification settings - Fork 3
Add support for running batch on a single lightcurve #420
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #420 +/- ##
==========================================
+ Coverage 95.82% 95.84% +0.02%
==========================================
Files 25 25
Lines 1771 1781 +10
==========================================
+ Hits 1697 1707 +10
Misses 74 74 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, just have one main change that I think should be made.
src/tape/ensemble.py
Outdated
@@ -1107,6 +1108,9 @@ def batch( | |||
source or object tables. If not specified, then the id column is | |||
used by default. For TAPE and `light-curve` functions this is | |||
populated automatically. | |||
single_lc: `int`, optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be really helpful to also allow single_lc=True
, where select_random_lightcurve is used to pick one for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense. To do that, I added an id_only
parameter to Ensemble.select_random_timeseries
so that it will only fetch the object id of a random lightcurve. But let me know if you would prefer that not added to the API and I can break that out differently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! That looks like a good solution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Change Description
In #409 it was proposed to add a batch helper function
try_batch
which would allowEnsemble.batch
to be tested on a single lightcurve. To accomplish that goal, here we instead add a new parametersingle_lc
toEnsemble.batch
When
single_lc
is provided an object ID, the function provided tobatch
is executed on that single lightcurve.Code Quality
New Feature Checklist