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

Updates to plugin-self-paced-reading #79

Merged
merged 37 commits into from
Oct 5, 2023
Merged

Conversation

jessestorbeck
Copy link
Contributor

This should close #75 and supersede #77.

  • Calls to jsPsych.data.write() are removed, and jsPsych.finishTrial() now gets an object with words and reading times stored as arrays. Each trial now generates a single data object.
  • A bug when inter_word_interval > 0 is corrected. Previously, the inter_word_interval value would be subtracted from the reading time of the initial blank and not subtracted from the reading time of the first word. This could lead to unrecognized valid keypresses and erroneous reading times, which become easier to see if you set inter_word_interval to an unrealistically long value. The main issue is here, keeping in mind that word_number is initialized at -1:
if (word_number === 0) {
    response.rt_word = rts[rts.length - 1] - rts[rts.length - 2];
} else {
    response.rt_word = rts[rts.length - 1] - rts[rts.length - 2] - trial.inter_word_interval;
}
  • Tests are added related to reading times and inter_word_interval. Equivalent tests would fail with the existing version. The tests have also been updated to remove extra keypresses, which were necessary before because they represented invalid RTs.
  • I'm not sure if yarn.lock should be in here or if it should be added to the .gitignore.

@changeset-bot
Copy link

changeset-bot bot commented Oct 2, 2023

🦋 Changeset detected

Latest commit: 465f58c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@jspsych-contrib/plugin-self-paced-reading Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jodeleeuw
Copy link
Member

Thanks @jessestorbeck !

I think we should remove yarn.lock, since the rest of the environment is using package.lock files. @bjoluc want to confirm that?

The data values are all prefixed with spr_ which isn't typically how plugins store data. Could we use rt, etc., instead? Is there a reason for the prefix?

@jodeleeuw jodeleeuw mentioned this pull request Oct 2, 2023
@jessestorbeck
Copy link
Contributor Author

@jodeleeuw -- There were other vars called words and rts that I was trying to avoid, but I see now that's unnecessary. I can update.

Copy link
Member

@jodeleeuw jodeleeuw left a comment

Choose a reason for hiding this comment

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

Just a few more tweaks and I think it's ready to go. 🚀

(Also remove yarn.lock as part of that update)

packages/plugin-self-paced-reading/src/index.ts Outdated Show resolved Hide resolved
packages/plugin-self-paced-reading/readme.md Outdated Show resolved Hide resolved
packages/plugin-self-paced-reading/readme.md Outdated Show resolved Hide resolved
@jessestorbeck
Copy link
Contributor Author

@jodeleeuw -- I believe I've addressed everything now. Thanks!

@jodeleeuw jodeleeuw merged commit 9afb139 into jspsych:main Oct 5, 2023
@github-actions github-actions bot mentioned this pull request Oct 5, 2023
@jodeleeuw
Copy link
Member

Thanks, @jessestorbeck!

This should be released on npm now.

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.

self-paced-reading doesn't record data
3 participants