Skip to content

FPM-566 Add support for alt data infilling #34

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
Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

FPM-566 Add support for alt data infilling #34

wants to merge 12 commits into from
+253 −21

Conversation

@dolegi
Copy link

@dolegi dolegi commented Oct 21, 2025 •
edited
Loading

This requires the alternative data to be part of the existing dataframe, maybe we want this to support alternative data from a different source/dataframe? @richjam what do you think?

@richjam
Copy link
Collaborator

richjam commented Oct 21, 2025

This requires the alternative data to be part of the existing dataframe, maybe we want this to support alternative data from a different source/dataframe? @richjam what do you think?

Potentially, yes. In the time series processor, each variable is held in it's own TimeFrame, so that would need this use case. I guess you could add option to send a new alt_data_df to the __init__, which if provided the _fill method will look for the alt_data_column in there. Would have to merge on the "time" column to make sure that the data exists at the same time points (which would mean we'd expect the name of the time column to be the same in the alt dataframe).

richjam
richjam requested changes Oct 22, 2025
View reviewed changes
Copy link
Collaborator

@richjam richjam left a comment

Choose a reason for hiding this comment

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

Happy to accept this as is for now, and have another ticket for the alt data coming from another source if you want.

But can you just add something to the user_guide/infilling.rst page as well?

dolegi
dolegi commented Oct 24, 2025
View reviewed changes
richjam
richjam requested changes Oct 27, 2025
View reviewed changes
docs/source/user_guide/infilling.rst Outdated
The :meth:`~time_stream.TimeFrame.infill` method is the entry point for infilling your
timeseries data in **Time-Stream**. It delegates to well established methods from the `SciPy data science library
<https://docs.scipy.org/doc/scipy/reference/interpolate.html>`_, combined with the time-integrity of your **TimeFrame**.
timeseries data in **Time-Stream**. There are various infill methods available; from using alternative data from
Copy link
Collaborator

Choose a reason for hiding this comment

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

Added suggested changes to this docs page (and associated "examples" code folder). Please can you review?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry where are these suggestions?

richjam
richjam requested changes Oct 27, 2025
View reviewed changes
Copy link
Collaborator

@richjam richjam left a comment

Choose a reason for hiding this comment

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

Unit tests failing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richjam richjam richjam requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dolegi @richjam