Skip to content

[FIX] Make the ea parser work without hardcoded stuff#292

Merged
josephmje merged 9 commits intoTIGRLab:masterfrom
DESm1th:master
Jan 19, 2021
Merged

[FIX] Make the ea parser work without hardcoded stuff#292
josephmje merged 9 commits intoTIGRLab:masterfrom
DESm1th:master

Conversation

@DESm1th
Copy link
Copy Markdown
Contributor

@DESm1th DESm1th commented Nov 27, 2020

This contains my and Thomas' updates and fixes a few bugs that were present.

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Nov 27, 2020

Hello @DESm1th, Thank you for updating!

Line 141:13: E126 continuation line over-indented for hanging indent
Line 420:10: E111 indentation is not a multiple of four
Line 420:10: E117 over-indented
Line 439:21: E126 continuation line over-indented for hanging indent
Line 439:81: E501 line too long (83 > 80 characters)

To test for issues locally, pip install flake8 and then run flake8 datman.

Comment last updated at 2021-01-19 18:50:20 UTC

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 27, 2020

Codecov Report

Merging #292 (ba3726a) into master (41d9c79) will decrease coverage by 0.21%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
- Coverage   32.44%   32.23%   -0.22%     
==========================================
  Files          60       60              
  Lines        8861     8919      +58     
==========================================
  Hits         2875     2875              
- Misses       5986     6044      +58     
Impacted Files Coverage Δ
bin/dm_parse_ea.py 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41d9c79...ba3726a. Read the comment docs.

jerdra
jerdra previously approved these changes Dec 8, 2020
@josephmje
Copy link
Copy Markdown
Contributor

@ThomasHMAC if the pandas version is different from the one in setup.cfg, could you please update that file as well

Copy link
Copy Markdown
Contributor

@jerdra jerdra left a comment

Choose a reason for hiding this comment

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

One more comment as I'm trying to run this script for MODSOCCS:

Can you update the script to be able to run participants individually? It's much easier to debug when you don't force a run of all participants. When one subject has an issue w/its EA data the whole script shouldn't have to fail (see comment w/gracefully handling a TypeError)

Comment thread bin/dm_parse_ea.py Outdated
Comment on lines +164 to +166
combo2["rating_duration"] = combo2["onset"].shift(-1) - combo2[
"onset"
].where(mask is False)
Copy link
Copy Markdown
Contributor

@jerdra jerdra Jan 19, 2021

Choose a reason for hiding this comment

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

Suggested change
combo2["rating_duration"] = combo2["onset"].shift(-1) - combo2[
"onset"
].where(mask is False)
combo2["rating_duration"] = combo2["onset"].shift(-1) - combo2[
"onset"
].where(mask == False)

I get an error w/pandas 1.0.3 since "mask is False" evaluates to a single element

Comment thread bin/dm_parse_ea.py
log_file = pd.read_csv(path, sep="\t", skiprows=3)

time_to_subtract=int(log_file.Duration[log_file.Code=='MRI_start'])
time_to_subtract = int(log_file.Duration[log_file.Code == "MRI_start"])
Copy link
Copy Markdown
Contributor

@jerdra jerdra Jan 19, 2021

Choose a reason for hiding this comment

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

doesn't gracefully handle TypeError where integer conversion cannot be performed (i.e missing MRI_start event)

Comment thread bin/dm_parse_ea.py
Comment on lines +248 to +252
logger.warning(
"gold standard is shorter than the number of pt "
"ratings. pt ratings truncated",
block_name,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.warning(
"gold standard is shorter than the number of pt "
"ratings. pt ratings truncated",
block_name,
)
logger.warning(
"gold standard is shorter than the number of pt "
f"ratings. pt ratings truncated, block: {block_name}"
)

Comment thread bin/dm_parse_ea.py
Comment on lines +256 to +260
logger.warning(
"number of pt ratings is shorter than the number "
"of gold std, gold std truncated",
block_name,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.warning(
"number of pt ratings is shorter than the number "
"of gold std, gold std truncated",
block_name,
)
logger.warning(
"number of pt ratings is shorter than the number "
f"of gold std, gold std truncated, block: {block_name}"
)

@jerdra
Copy link
Copy Markdown
Contributor

jerdra commented Jan 19, 2021

@josephmje i've made some updates to the script to resolve my own comments, can you do a quick review whenever you get the chance? Mostly so we can get it within the code-base for SPN20 QC?

This script needs a full re-write eventually, its barely maintainable 🤷

Thanks Dawn/Thomas for dealing with this beast for the most part!

@josephmje josephmje merged commit ef9563a into TIGRLab:master Jan 19, 2021
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.

5 participants