Skip to content

Comments

NF: Adding field seperator selection in output#566

Open
EitanHemed wants to merge 8 commits intopsychopy:mainfrom
EitanHemed:delimiter_feature
Open

NF: Adding field seperator selection in output#566
EitanHemed wants to merge 8 commits intopsychopy:mainfrom
EitanHemed:delimiter_feature

Conversation

@EitanHemed
Copy link

Following this feature request, this pull request will allow passing the data delimiter selected on PsychoPy, when converting the experiment to PsychoJS.

Note that the changes are not implemented yet on the PsychoPy end, and psychoJS.ExperimentHandler._field_separator is an undefined property. Thus the expected behavior is that the default (',') field separator will be selected.

Adding to ExperimentHandler a field separator property, which allows output to include field separators other than comma (e.g., TSV format).
Fixed type in this._field_separator
Passing parameters correctly to .sheet_to_csv
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.

Broadly I like your thinking, but I think it's better for cross-language parity if the delim is an input when creating the experiment handler rather than being in expInfo. Being in expInfo means that it will appear in the data file and (if we don't otherwise handle it) the participant info dialog.

What if instead of looking for field_separator in extraInfo, ExperimentHandler simply had an attribute .field_separator (like what you extract to here) which defaults to "," if not otherwise set? Then in the compiled code from the app we can do psychoJS.experiment.field_separator = %(Data file delimiter)s

@EitanHemed
Copy link
Author

Thanks for the feedback @TEParsons, i think the current changes should do it and not putting it in expInfo is more elegant.
BTW, as far as i could see entries in expInfo which begin with an underscore are excluded from the participant info dialog.

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.

Spot on, this looks great!

I've tested the Python end and confirmed the code is generated fine, @apitiot @lightest is there a way to test the JS end that this value is used correctly by the experiment handler?

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