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

Added NonSequentialTimeSeries Model #76

Merged
merged 5 commits into from
Mar 6, 2025
Merged

Added NonSequentialTimeSeries Model #76

merged 5 commits into from
Mar 6, 2025

Conversation

abodh
Copy link
Collaborator

@abodh abodh commented Feb 28, 2025

-Added NonSequentialTimeSeriesModel which can store non-sequential time steps as contrary to SingleTimeSeries which works with a fixed resolution of time steps

  • Updated existing modules coupled with SingleTimeSeries to be generic such that they can now accept NonSequentialTimeSeries model
  • Replicated tests as applicable to SingleTimeSeries
  • In file storage is stored as a pyarrow.StructArray contrary pyarrow.array as we store data and timestamps in different types. This can also follow a pandas.DataFrame approach but I think parsing two different arrays -- one for data and one for timestamps -- should be fine for now.

-Added NonSequentialTimeSeriesModel which can store NonSequential time steps as contrary to SingleTimeSeries which works with a fixed resolution of time steps
- Updated existing modules coupled with SingleTimeSeries to be generic such that they can now accept NonSequentialTimeSeries model
- Replicated tests as applicable to SingleTimeSeries
- Infile storage is stored as a StructArray contrary pyarrow array as we store data and timestamps in different types
@abodh abodh added the enhancement New feature or request label Feb 28, 2025
@abodh abodh requested a review from daniel-thom February 28, 2025 23:06
@abodh abodh linked an issue Feb 28, 2025 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2025

Codecov Report

Attention: Patch coverage is 92.11196% with 31 lines in your changes missing coverage. Please review.

Project coverage is 95.47%. Comparing base (ea02654) to head (6056c2d).

Files with missing lines Patch % Lines
src/infrasys/time_series_models.py 80.43% 18 Missing ⚠️
src/infrasys/arrow_storage.py 90.00% 5 Missing ⚠️
src/infrasys/in_memory_time_series_storage.py 77.27% 5 Missing ⚠️
src/infrasys/time_series_manager.py 81.81% 2 Missing ⚠️
src/infrasys/time_series_metadata_store.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #76      +/-   ##
==========================================
- Coverage   95.77%   95.47%   -0.30%     
==========================================
  Files          46       47       +1     
  Lines        3835     4179     +344     
==========================================
+ Hits         3673     3990     +317     
- Misses        162      189      +27     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

abodh added 2 commits March 5, 2025 10:03
- updated and resolved changes from the main branch
- updated the arrow storage to handle record array instead of pyarrow structure
- complied with the TimeSeriesKey model for NonSequentialTimeSeries
- merged with the main branch with Chronify-related changes
- resolved conflicts with incoming changes and merged changes reflecting
  NonSequentialTimeSeries
- pytest complies with all the merged changes
@abodh abodh self-assigned this Mar 5, 2025
Copy link
Collaborator

@daniel-thom daniel-thom left a comment

Choose a reason for hiding this comment

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

It looks good now. I have a few minor comments.

@@ -221,7 +221,7 @@ def to_input_output(

xs = self.function_data.x_coords
ys = np.multiply(xs[1:], self.function_data.y_coords).tolist()
ys.insert(0, c)
ys.insert(0, c) # type:ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pesap Do you know why we need this?

- added chronological checks based on comments from CADET team
- merged add time series methods into one based on singleedispatchmethod
  func
- os and path permission error on windows
@daniel-thom daniel-thom merged commit d60d973 into main Mar 6, 2025
6 checks passed
@daniel-thom daniel-thom deleted the ap/issue71 branch March 6, 2025 00:08
github-actions bot pushed a commit that referenced this pull request Mar 6, 2025
Added NonSequentialTimeSeries Model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a new time series model to handle NonSequentialTimeSeries data
3 participants