Skip to content

Supporting non contiguous domain on RegularTimeseries#40

Draft
AlexandreAndr wants to merge 31 commits intomainfrom
aandre/fix_regular_non_contiguous
Draft

Supporting non contiguous domain on RegularTimeseries#40
AlexandreAndr wants to merge 31 commits intomainfrom
aandre/fix_regular_non_contiguous

Conversation

@AlexandreAndr
Copy link
Copy Markdown
Member

@AlexandreAndr AlexandreAndr commented Apr 19, 2025

Pull Request Description: Support for Non-Contiguous Domains on RegularTimeSeries and LazyRegularTimeSeries

Overview

This pull request introduces support for non-contiguous domains with RegularTimeSeries to handle disjoint intervals, improving slicing operations, domain calculations, and timestamp generation for complex datasets.

Key Changes

  1. Non-Contiguous Domain Support:

    • Updated the RegularTimeSeries class to handle disjoint intervals in the domain.
    • Enhanced slicing functionality (slice method) to work seamlessly with non-contiguous domains, ensuring accurate extraction of data across multiple intervals.
    • Improved timestamp calculations to reflect the non-contiguous structure.
  2. Expanded Test Suite:

    • Added comprehensive test cases to validate the handling of non-contiguous domains in slicing and timestamp generation.
    • Covered edge cases, including skewed intervals, overlapping intervals, and slices that extend beyond the domain boundaries.
    • Enhanced existing tests to ensure correctness and compatibility with the new logic.
  3. Refinements to Mask Creation:

    • Improved the add_split_mask test on other object IrregularTimeSeries (can be moved to another PR).

Testing

  • Thoroughly tested with new and expanded test cases covering a wide range of scenarios.
  • Validated against edge cases to ensure numerical precision and proper handling of disjoint intervals.

Here is an example of a RegularTimeSeries with non contiguous domains:

# Example of non continous domain
data = RegularTimeSeries(
    raw=np.zeros((2000, 128)),
    sampling_rate=250.0,
    domain=Interval(start=np.array([0.0, 4.0]), end=np.array([5.9, 9.9])),
)

Copilot AI review requested due to automatic review settings April 19, 2025 20:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces tests to support non contiguous domains in RegularTimeSeries objects. Key changes include the addition of a new test function for non contiguous domains, adjustments in import ordering, and updates to the collection of imported symbols.

Comments suppressed due to low confidence (3)

tests/test_data.py:820

  • [nitpick] The variable 'regular_timseries' appears to be misspelled. Consider renaming it to 'regular_timeseries' for clarity, and ensure that 'base_sampling_rate' and 'v' are defined before use.
regular_timseries = RegularTimeSeries(

tests/test_data.py:815

  • The variable 'b' is used but not defined in this test. Consider replacing 'b' with 'a' if the intended object is the one instantiated above.
assert np.allclose(b.timestamps, np.arange(0, 10, 0.1))

tests/test_data.py:818

  • The variables 'start' and 'end' are not defined in this context. Define these values or use the appropriate variables to represent the intended interval boundaries.
domain = Interval(start, end)

@AlexandreAndr AlexandreAndr marked this pull request as draft February 27, 2026 05:17
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.

3 participants