Skip to content

Conversation

wulfdewolf
Copy link
Collaborator

@wulfdewolf wulfdewolf commented Jul 3, 2025

This combines:

  • compute_1d_tuning_curves
  • compute_1d_tuning_curves_continuous
  • compute_2d_tuning_curves
  • compute_2d_tuning_curves_continuous

into a generalised n-dimensional compute_tuning_curves, which now returns an xarray.DataArray (new dependency).

The old functions are still available, but they call the new one under the hood and show a DeprecationWarning.
A couple of notes (these clarify why I had to make some changes to the old tuning curve function tests):

  • for compute_1d_tuning_curves, we were using features.rate to scale the tuning curves to Hz, but if one passes epochs that are bigger than that of the input features, this results in an incorrect computation of the rate and thus incorrect tuning curves.
    FIXES: we now compute the rate as 1/np.mean(features.time_diff(epochs)).
    There further is an optional fs argument, for if you want to pass the exact sampling rate yourself.

  • for compute_1d_tuning_curves_continuous and compute_2d_tuning_curves_continuous, we were using np.digitize to bin, but this does not include the rightmost edge , unlike np.histogram.
    FIXES: we now use np.histogram for all tuning curves.
    Hence, the new function is more consistent, as all tuning curves will now include the rightmost edge.

I have further updated all the notebooks and docs to use the generalised function.
The new xarray.DataArray output allows for some more concise plotting in a lot of the notebooks.

At the moment, the decoding is still separated into 1D and 2D, generalising that will be another PR.
For now, I am transforming the xarray.DataArrays into the respective old formats, such that the decoding functions still work.

@wulfdewolf wulfdewolf linked an issue Jul 3, 2025 that may be closed by this pull request
note as to why I changed some original tests:
the old compute_2d_tuning_curves_continuous used np.digitize, which does
not include the last edge
the new implementation does, so I had to reflect that in the expected
values
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
pynapple/core/interval_set.py 97.11% <ø> (ø)
pynapple/core/ts_group.py 92.88% <ø> (ø)
pynapple/process/__init__.py 100.00% <ø> (ø)
pynapple/process/tuning_curves.py 95.91% <100.00%> (-0.17%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wulfdewolf wulfdewolf self-assigned this Jul 8, 2025
@wulfdewolf wulfdewolf requested a review from sjvenditto July 9, 2025 20:27
@wulfdewolf wulfdewolf marked this pull request as ready for review July 9, 2025 20:33
@wulfdewolf wulfdewolf requested a review from sjvenditto July 16, 2025 18:41
Copy link
Collaborator

@sjvenditto sjvenditto left a comment

Choose a reason for hiding this comment

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

I've left a few final comments that are more inconsequential, so let me know your thoughts on if you want to put in these small changes -- otherwise, this one's just about done!

@wulfdewolf wulfdewolf requested a review from sjvenditto July 24, 2025 21:42
@wulfdewolf wulfdewolf requested a review from gviejo July 29, 2025 15:23
@gviejo gviejo merged commit 7bca9e4 into dev Jul 30, 2025
16 checks passed
@wulfdewolf wulfdewolf deleted the 477-tc-generalisation branch July 30, 2025 22:36
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.

Generalising tuning curves to n dimensions

3 participants