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

bugfix: pass down trial parameter to subclass. #276

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pellet
Copy link
Contributor

@pellet pellet commented Oct 2, 2024

bugfix: pass down trial parameter to subclass as was previously done before VR code modified rendering logic.

Also fixed baseclass 'present_stimulus' method signature to match arguments as expected for subclass to implement.

This was something which I accidentally had modified the behaviour of - I had trouble understanding what the 'trial' parameter was used for in the experiments.
It seems like none of the example experiments are using the parameter.

@JohnGriffiths
Copy link
Collaborator

@pellet - summary from discussion:

looks like the trial variable actually wasn't ever being used

could you please first check that in the rest of the code and confirm

if so, update this PR to remove that variable entirely as it's redundant

thanks

@pellet pellet force-pushed the dev/fix_present_stimulus branch from c73895c to 8275489 Compare November 10, 2024 05:38
@pellet pellet changed the base branch from develop to master November 10, 2024 05:38
@pellet pellet force-pushed the dev/fix_present_stimulus branch from 8275489 to e6dc15e Compare November 10, 2024 05:40
@JohnGriffiths
Copy link
Collaborator

@pellet as discussed - I want to give this a spin in a few more contexts before merging.

I think you're right that this does leave things in a better state than they currently are.

Noting - the main issue to consider here now is actually not the trial parameter, but the behaviour of the audio device, which is also fixed in this PR. But, it was behaving weirdly for me in testing just now.

And it looks like the audio device in general may have suffered some recent breakages we haven't yet fully clocked.

@pellet
Copy link
Contributor Author

pellet commented Dec 17, 2024

This last change I made got the auditory oddball experiment running ok on a mac running python as i386(Roslyn). I have avoided importing the experiment when running on macOS ARM to prevent it from crashing. PTB looks to be adding official support for macOS ARM sometime around January but will also begin charging licenses from version 3.0.20 on.

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