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

Implementing build_tensor and warp_tensor in warping.py #388

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

gviejo
Copy link
Contributor

@gviejo gviejo commented Jan 14, 2025

Here is the proposed behavior :

Simple trial-based tensor :

import pynapple as nap
import numpy as np
group = nap.TsGroup({0:nap.Ts(t=np.arange(0, 100))})
ep = nap.IntervalSet(start=np.arange(20, 100, 20), end=np.arange(20, 100, 20) + np.arange(2, 10, 2))
tensor = nap.build_tensor(group, ep, binsize=1)
print(tensor)
    array([[[ 1.,  1., nan, nan, nan, nan, nan, nan],
            [ 1.,  1.,  1.,  1., nan, nan, nan, nan],
            [ 1.,  1.,  1.,  1.,  1.,  1., nan, nan],
            [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.]]])

Time-warped trial based tensor :

import pynapple as nap
import numpy as np
group = nap.TsGroup({0:nap.Ts(t=np.arange(0, 100))})
ep = nap.IntervalSet(start=np.arange(20, 100, 20), end=np.arange(20, 100, 20) + np.arange(2, 10, 2))
tensor = nap.warp_tensor(group, ep, num_bin=10)
print(tensor)
    array([[[1., 0., 0., 0., 0., 1., 0., 0., 0., 0.],
            [1., 0., 1., 0., 0., 1., 0., 1., 0., 0.],
            [1., 1., 0., 1., 0., 1., 1., 0., 1., 0.],
            [1., 1., 1., 1., 0., 1., 1., 1., 1., 0.]]])

Both functions works for any time series.

Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 89.00000% with 22 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pynapple/core/time_series.py 73.33% 6 Missing and 6 partials ⚠️
pynapple/core/ts_group.py 68.75% 5 Missing and 5 partials ⚠️
Files with missing lines Coverage Δ
pynapple/core/base_class.py 97.63% <100.00%> (ø)
pynapple/process/__init__.py 100.00% <100.00%> (ø)
pynapple/process/perievent.py 100.00% <100.00%> (+5.20%) ⬆️
pynapple/process/warping.py 100.00% <100.00%> (ø)
pynapple/core/ts_group.py 90.65% <68.75%> (-1.91%) ⬇️
pynapple/core/time_series.py 88.36% <73.33%> (-1.07%) ⬇️

... and 1 file with indirect coverage changes

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

@sjvenditto
Copy link
Collaborator

I have some thoughts about the naming conventions used here:

  1. Having warp_tensor as a separate function from build_tensor feels a little misleading since warp_tensor also "builds" a tensor.

My thought is to have a central function build_tensor with a method parameter that can be "pad" or "warp", that takes method_kwargs, so something like:

def build_tensor(input, ep, method, **method_kwargs)

and that way both ways of building a tensor can be accessed in a centralized location. In this way, the current build_tensor would be renamed to pad_tensor, and keep the kwargs definitions in the docstring. warp_tensor could remain as-is

  1. Similar to numpy functions like np.reshape, I think it would be useful to define an object method to_tensor that calls build_tensor

What do you think about this?

@gviejo
Copy link
Contributor Author

gviejo commented Mar 6, 2025

I would have build_tensor as a method for Tsds and TsGroup but warp_tensor as a function of warping.py. I can see a path where more warping methods are added inflating the core while build_tensor with simple padding should remain small.

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.

2 participants