Skip to content

Conversation

@EitanHemed
Copy link
Contributor

This comes to solve issue #5141, a feature-request to be able to pass the data file separator to psychoJS (currently uses only comma).

The changes proposed here should be reviewed alongside this modification.

Copy link
Contributor

@TEParsons TEParsons left a comment

Choose a reason for hiding this comment

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

See my comments on the accompanying PsychoJS PR for feedback on using a field in expInfo

It's also worth noting that this would only appear in the compiled Python code - the functions to write JS code all end with JS (e.g. writeInitCodeJS)

@TEParsons TEParsons self-requested a review July 3, 2023 10:31
@TEParsons
Copy link
Contributor

Looks great! The test suite is failing because you're using the Param object to index delim_options rather than its (string) value - easy mistake to make and very easy to fix. I'll submit a fix to your branch, once that's merged I'm happy to pull in both this and (pending approval from the JS team) the accompanying PsychoJS PR

@TEParsons
Copy link
Contributor

Oh, never mind, looks like I can push directly to your branch... So yes, once the accompanying PsychoJS PR is approved I'll pull this one in too :)

@peircej peircej merged commit 7df8447 into psychopy:dev Jul 18, 2023
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.

3 participants