Change output filename & ScreenResolution return values #34
Conversation
julia-pfarr
commented
Oct 25, 2023
julia-pfarr
commented
- output filenames: according to issue output_dir + filename #24: output file has now the prefix of the edf file. If edf file already contains "_eyetrack" suffix, it is not written again.
- ScreenResolution threw an error because it expected integers but it returned strings --> changed to integers
- output filenames: according to issue #24: output file has now the prefix of the edf file. If edf file already contains "_eyetrack" suffix, it is not written again. - ScreenResolution threw an error because it expected integers but it returned strings --> changed to integers
for more information, see https://pre-commit.ci
|
Included code to extract eyetrack.tsv from samples_asc @Remi-Gau like I already said in the BIDS chat group, the tests are failing and and the output_dir is not considered when I run eye2bids in the terminal (i.e., I define --output_dir but my output is saved in my input_file directory anyway). I don't know why this is, apparently I changed something that caused this. But I don't think I can find out what it is |
|
side note: tests were already failing in #33 so I will probably start fixing things in main and then get back to this PR. |
Yes but that was due to a different thing which I fixed with this PR (it was ScreeResolution expected int and got str, which I fixed in this PR. Now it's a different error) |
| SampleCoordinateUnits: !!str | ||
| SampleCoordinateSystem: !!str | ||
| EnvironmentCoordinates: !!str | ||
| ScreenDistance: !!int | ||
| ScreenRefreshRate: !!int | ||
| ScreenSize: [!!float] | ||
| SampleCoordinateUnits: str | ||
| SampleCoordinateSystem: str | ||
| EnvironmentCoordinates: str | ||
| ScreenDistance: int | ||
| ScreenRefreshRate: int | ||
| ScreenSize: [float] | ||
|
|
||
| # Recommended (leave empty if not available but put information you want to share with your dataset here!): | ||
| SoftwareVersion: !!str | ||
| ScreenAOIDefinition: [!!str [!!int]] | ||
| SoftwareVersion: str | ||
| ScreenAOIDefinition: [str, [int]] | ||
| EyeCameraSettings: | ||
| EyeTrackerDistance: !!float |
There was a problem hiding this comment.
this was breaking the parsing of the yml file
eye2bids/edf2bids.py
Outdated
| if substring_eyetrack not in filename: | ||
| with open(output_dir / (filename + "_eyetrack.tsv"), "w") as outfile: | ||
| eyetrack_tsv.to_csv(outfile, sep="\t", index=False, compression="gzip") | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '_eyetrack.tsv')}") | ||
| else: | ||
| with open(output_dir / (filename + ".tsv"), "w") as outfile: | ||
| eyetrack_tsv.to_csv(outfile, sep="\t", index=False, compression="gzip") | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '.tsv')}") |
eye2bids/edf2bids.py
Outdated
| if substring_events not in filename: | ||
| with open(output_dir / (filename + "_events.json"), "w") as outfile: | ||
| json.dump(events_json, outfile, indent=4) | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '_events.json')}") | ||
| else: | ||
| with open(output_dir / (filename + ".json"), "w") as outfile: | ||
| json.dump(events_json, outfile, indent=4) | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '.json')}") |
eye2bids/edf2bids.py
Outdated
| # to json | ||
| # file naming check | ||
|
|
||
| filename = Path(input_file).stem |
There was a problem hiding this comment.
changed this line to make sure that the file would be created in the correct folder
| @@ -7,7 +7,7 @@ | |||
|
|
|||
| from eye2bids.edf2bids import ( | |||
| _check_edf2asc_present, | |||
| _convert_edf_to_asc, | |||
There was a problem hiding this comment.
tests were failing because you had not renamed this function here and it could not be imported
There was a problem hiding this comment.
AHA!!!! Thanks for explaining!
There was a problem hiding this comment.
well more precisely the test could not even start running because of that
now I am running them and figuring out why se get some failures
eye2bids/edf2bids.py
Outdated
| if substring_eyetrack not in filename: | ||
| with open(output_dir / (filename + "_eyetrack.tsv"), "w") as outfile: | ||
| eyetrack_tsv.to_csv(outfile, sep="\t", index=False, compression="gzip") | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '_eyetrack.tsv')}") | ||
| else: | ||
| with open(output_dir / (filename + ".tsv"), "w") as outfile: | ||
| eyetrack_tsv.to_csv(outfile, sep="\t", index=False, compression="gzip") | ||
| e2b_log.info(f"file generated: {output_dir / (filename + '.tsv')}") |
There was a problem hiding this comment.
I don't understand why we may have those different cases of filenaming
There was a problem hiding this comment.
@mszinte had a dataset where he already had named the files with "_eyetrack" at the end. I don't know if this is super common but I guess it could be that others had the same idea of naming their files like this without knowing BIDS. Then when they want to convert it to BIDS they would get "_eyetrack_eyetrack" as output filename
There was a problem hiding this comment.
OK fair, I will refactor the code a bit for this rare case
| pytest.skip("edf2asc missing") | ||
|
|
||
| input_dir = eyelink_test_data_dir / "decisions" | ||
| input_dir = eyelink_test_data_dir / "satf" |
There was a problem hiding this comment.
There is one EDF file in the "decision" dataset that fails.
Need to figure out why: maybe not in this PR though
|
@julia-pfarr Can you check that the data is saved in the appropriate folder? |
yes, data is saved in the appropriate folder |
