Skip to content

Updated EA parse to remove button press during scan response #298

Merged
DESm1th merged 5 commits intoTIGRLab:masterfrom
ThomasHMAC:master
Mar 12, 2021
Merged

Updated EA parse to remove button press during scan response #298
DESm1th merged 5 commits intoTIGRLab:masterfrom
ThomasHMAC:master

Conversation

@ThomasHMAC
Copy link
Copy Markdown
Contributor

No description provided.

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Feb 12, 2021

Hello @ThomasHMAC, Thank you for updating!

Line 52:81: E501 line too long (97 > 80 characters)
Line 59:81: E501 line too long (112 > 80 characters)
Line 60:81: E501 line too long (91 > 80 characters)
Line 77:81: E501 line too long (104 > 80 characters)
Line 103:81: E501 line too long (94 > 80 characters)
Line 185:44: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 212:14: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 431:5: F841 local variable 'e' is assigned to but never used
Line 436:1: W293 blank line contains whitespace

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

Comment last updated at 2021-02-22 14:44:01 UTC

Comment thread bin/dm_parse_ea.py
Comment on lines +57 to +62
for index, row in log_file.iterrows():
if ("rating" in log_file["Code"][index]) and any(
resp in log_file["Code"][index - 2] for resp in scan_response
):
indexes_to_drop.append(index)
indexes_to_drop.append(index - 2)
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.

Can you add a comment describing why the current index and 2 indices beforehand need to be dropped? Mostly so we don't have a magic number in there that people in the future wouldn't understand!

Comment thread bin/dm_parse_ea.py
Comment on lines +69 to +71
logger.warning(
"Removed registered rating occurred before or after actual rating"
)
Copy link
Copy Markdown
Contributor

@jerdra jerdra Feb 12, 2021

Choose a reason for hiding this comment

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

Maybe include the number of "chunks" being dropped here for debugging purposes (i.e if an abnormally large number of indices got dropped).

Suggested change
logger.warning(
"Removed registered rating occurred before or after actual rating"
)
logger.warning(
f"Removed {len(indexes_to_drop)/2} registered ratings that occurred before or after actual rating"
)

Comment thread bin/dm_parse_ea.py
Comment on lines +73 to +81
indexes_to_drop_1 = []

for index, row in log_file_cleaned.iterrows():
if ("rating" in log_file_cleaned["Code"][index]) and any(
resp in log_file_cleaned["Code"][index - 1]
for resp in scan_response
):
indexes_to_drop_1.append(index)
indexes_to_drop_1.append(index - 1)
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.

I think a "why" comment is needed here as well, might just be me, but I'm not really sure why we dropped i, i-2, and now i,i-1!

Comment thread bin/dm_parse_ea.py Outdated
Comment on lines +89 to +90
final_log_file = final_log_file.reset_index(drop=True)
logger.warning("Removed rating registered followed scanner responses")
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.

Same as above logging comment re: including the number of ratings dropped

Comment thread bin/dm_parse_ea.py Outdated
df["duration"] = df["movie_name"].apply(
lambda x: int(vid_info[x]["duration"]) * 10000
if x in vid_info
else "n/a"
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.

maybe it might be a good idea to use to use pd.NA here to avoid having a mixed data-type column

Comment thread bin/dm_parse_ea.py Outdated
)

df["stim_file"] = df["movie_name"].apply(
lambda x: vid_info[x]["stim_file"] if x in vid_info else "n/a"
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.

same as above pd.NA comment

Comment thread bin/dm_parse_ea.py
# Reads in and clean the log, skipping the first three preamble lines
try:
log = read_in_logfile(log_file)
log_cleaned = clean_logfile(log)
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.

could this be outside the try/catch block? It looks like clean_logfile is mostly if not entirely safe

Comment thread bin/dm_task_files.py
Comment on lines +49 to +52
try:
os.mkdir(dest_folder)
except OSError:
pass
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
try:
os.mkdir(dest_folder)
except OSError:
pass
os.makedirs(dest_folder, exist_ok=True)

Unless you want it to error out if intermediate directories don't exist!

@DESm1th DESm1th self-requested a review March 12, 2021 20:06
@DESm1th DESm1th merged commit c5cf5c0 into TIGRLab:master Mar 12, 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