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.

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

Fix copy-view issue in epochs #12121

Merged
merged 16 commits into from
Nov 9, 2023
Merged

Fix copy-view issue in epochs #12121

Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5bd0dac
Fix copy-view issue in epochs
pablomainar Oct 21, 2023
f0f7173
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 21, 2023
15816db
Add PR suggestions
pablomainar Oct 25, 2023
98b01b6
Adapt tests to new interface
pablomainar Nov 1, 2023
19a30da
Merge branch 'main' into fix
drammock Nov 2, 2023
6436a68
Add tests for copy
pablomainar Nov 2, 2023
d740c31
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 2, 2023
1346645
Use shares_memory in test
pablomainar Nov 3, 2023
d310732
Merge branch 'main' into fix
larsoner Nov 3, 2023
69ca518
FIX: Close
larsoner Nov 8, 2023
31c299b
Merge remote-tracking branch 'upstream/main' into fix
larsoner Nov 8, 2023
a0c7ade
FIX: More
larsoner Nov 8, 2023
09792b5
FIX: Green [circle full]
larsoner Nov 8, 2023
1135dd9
Merge remote-tracking branch 'upstream/main' into fix
larsoner Nov 8, 2023
01e1d9c
FIX: Examples
larsoner Nov 9, 2023
b2e7c46
FIX: Simpler
larsoner Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions mne/epochs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1588,6 +1588,7 @@ def _get_data(
units=None,
tmin=None,
tmax=None,
copy=True,
Copy link
Member

Choose a reason for hiding this comment

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

in this private method we should probably default to copy=False so that we're not making unnecessary copies in internal calls that use the private API.

on_empty="warn",
verbose=None,
):
Expand Down Expand Up @@ -1672,6 +1673,9 @@ def _get_data(
if (units is not None) and out:
ch_factors = _get_ch_factors(self, units, picks)

if copy:
data = data.copy()
Copy link
Member

Choose a reason for hiding this comment

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

I suggest moving this up about 25 lines, inside the if branch of the if self.preload conditional. The else branch defines data = np.empty(...) so that will never be a view of the object's data and thus the copy will never be necessary.


if self._bad_dropped:
if not out:
return
Expand Down Expand Up @@ -1796,7 +1800,9 @@ def _detrend_picks(self):
return []

@fill_doc
def get_data(self, picks=None, item=None, units=None, tmin=None, tmax=None):
def get_data(
self, picks=None, item=None, units=None, tmin=None, tmax=None, copy=True
):
"""Get all epochs as a 3D array.

Parameters
Expand All @@ -1821,13 +1827,20 @@ def get_data(self, picks=None, item=None, units=None, tmin=None, tmax=None):
End time of data to get in seconds.

.. versionadded:: 0.24.0
copy : bool | None
If true (default) then a copy is returned. If false,
a view is returned if the requirements are met.
If picks, item, tmin or tmax are not None, a copy
is returned.
larsoner marked this conversation as resolved.
Show resolved Hide resolved

Returns
-------
data : array of shape (n_epochs, n_channels, n_times)
A view on epochs data.
"""
return self._get_data(picks=picks, item=item, units=units, tmin=tmin, tmax=tmax)
return self._get_data(
picks=picks, item=item, units=units, tmin=tmin, tmax=tmax, copy=copy
)

@verbose
def apply_function(
Expand Down
Loading