DOC: document first_samp behavior after crop and RawArray workaround#13685
DOC: document first_samp behavior after crop and RawArray workaround#13685larsoner merged 18 commits intomne-tools:mainfrom
Conversation
Famous077
requested review from
agramfort,
drammock,
larsoner and
mscheltienne
as code owners
|
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
for more information, see https://pre-commit.ci
|
Hi @larsoner , Added reset_first_samp=False parameter to Raw.crop() as suggested. |
|
Hi @mscheltienne , Could you please take a look when you have time? I’d really value your perspective and any suggestions for improvement. |
| If True, reset :term:`first_samp` to 0 after cropping, treating | ||
| the cropped segment as an independent recording. Note that this | ||
| can break things if you extracted events before cropping and try | ||
| to use them afterward. Default is False. |
There was a problem hiding this comment.
Hi @larsoner, thank you for the review. I have addressed all the feedbacks:
- Renamed changelog to
13685.other.rst - Added
.. versionadded:: 1.12toreset_first_sampdocstring - Added regression test
test_crop_reset_first_samp - Updated changelog to use
:newcontrib:format - Disclosed AI usage in the PR description
for more information, see https://pre-commit.ci
|
@Famous077 if you used AI tools during the development of these changes, could you let us know how you used them? |
|
Hi @larsoner, Thank you for asking. I used an AI tools mostly for Understanding the codebase, Docstring formatting, Test structure. But, All core decisions like what to implement, what the correct behavior should be, and whether the fix makes sense and requesting for feedback to update according to the suggestion that i get were all made by me. If any of my changes introduce unintended side effects, violate codebase conventions, or create a negative impact in any way, please do not hesitate to point them out directly or close the PR. I will completely understand and take it as a valuable learning experience. I apologize in advance if anything I have done has caused any inconvenience. |
Looks like this one hasn't been done yet ☝️ |
…ous077/mne-python into doc/crop-first-samp-behavior
for more information, see https://pre-commit.ci
Hi @larsoner , Sorry for the delay due to exams. I have implemented updated example as per your suggestion. Can you check and let me know changes are working correctly or not. Waiting for your feedback and learn more. |
for more information, see https://pre-commit.ci
|
Thanks @Famous077 ! |
|
Love to learn more from you @larsoner. |
2 participants
Reference issue (if any)
Fixed #13278
-->
What does this implement/fix?
Adds a reset_first_samp=False parameter to Raw.crop() that allows
users to treat cropped segments as independent recordings. When set
to True, first_samp is reset to 0 after cropping.
Also adds a Notes section to the docstring explaining the default
first_samp behavior after cropping and documents the RawArray
workaround for users who prefer that approach.
Additional information
As noted in the docstring, setting reset_first_samp=True can break
things if events were extracted before cropping and used afterward.
This is clearly documented in the parameter description to warn users.
The default is False to maintain backward compatibility with existing
code.