329 refactor joined timeseries class to allow adding fields #360
Conversation
There was a problem hiding this comment.
Looks good to me, just had a few questions/comments. (the reference to the added notebook in the user guide index is broken)
Another thought on the user guide is to add some content describing the difference between using .add_calculated_fields() vs. execute_udfs=True. It seems like executing the UDF's from the script makes it easier for the user to add their own custom functions?
We could also list the currently available RCF's and TCF's in the User Guide?
| @@ -38,6 +38,7 @@ arch = "^7.0.0" | |||
| pandera = {extras = ["pyspark"], version = "^0.20.4"} | |||
| netcdf4 = "1.6.5" | |||
| bokeh = "^3.5.0" | |||
There was a problem hiding this comment.
I guess we'll remove bokeh as a dependency in the future?
There was a problem hiding this comment.
Yeah, probably. I think it will still be installed but as a dep to holoviews, not directly.
| PercentileEventDetection = PercentileEventDetection | ||
|
|
||
| # print all properties such as PercentileEventDetection | ||
| def __repr__(self): |
There was a problem hiding this comment.
So this is a convenience function, correct? Just to be able to visualize the object?
There was a problem hiding this comment.
Good catch. Should remove that. I thought it would be nice if printing the object showed what "methods" it had, but it didn't work as I wanted.
| @@ -28,6 +28,8 @@ Before starting, make sure you have installed TEEHR and its dependencies as desc | |||
|
|
|||
| :doc:`Grouping and Filtering </user_guide/notebooks/06_grouping_and_filtering>` :download:`(download notebook) </user_guide/notebooks/06_grouping_and_filtering.ipynb>` | |||
|
|
|||
| :doc:`Adding calculated fields </user_guide/notebooks/08_adding_calculated_fields>` :download:`(download notebook) </user_guide/notebooks/08_adding_calculated_fields.ipynb>` | |||
There was a problem hiding this comment.
I think the notebook reference should be 08_add_calculated_fields (or change name of notebook?)
| @@ -40,4 +42,5 @@ Before starting, make sure you have installed TEEHR and its dependencies as desc | |||
| Read an Evaluation from S3 </user_guide/notebooks/07_read_from_s3> | |||
| Joining Timeseries </user_guide/tutorials/joining_timeseries> | |||
| Grouping and Filtering </user_guide/notebooks/06_grouping_and_filtering> | |||
| Adding calculated fields </user_guide/notebooks/08_adding_calculated_fields> | |||
Yeah, that probably makes sense to do.
Good idea. Maybe just in the notebook page. |
This PR adds the ability to add calculated fields (used to be called user defined fields, but that isn't really accurate anymore). Notably:
RowLevelCalculatedFieldsandTimeseriesAwareCalculatedFieldswhich are hopefully descriptive enough names.hvplotdependency to poertyadd_calculated_fields()methods tojoined_timeseriesandmetrics"tables"