Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning in app.R due to name collision? #65

Open
nohelix opened this issue May 16, 2023 · 5 comments
Open

Warning in app.R due to name collision? #65

nohelix opened this issue May 16, 2023 · 5 comments

Comments

@nohelix
Copy link
Collaborator

nohelix commented May 16, 2023

Likely Problem:
Direct issue is on line 351. However actual problem is in id_good (line 201) where there is NOT a check for active so reused colors will turn up multiple matches.

Solution:
On line 203 add
rat_archive %>% dplyr::filter(str_to_upper(Rat_name) == name_good() & is.na(end_date)) %>% .$Rat_ID
Note: If somehow there are multiple rats with the same name, which shouldn't happen, this error will still occur.

Rats giving warnings:
Teal's, Red's, Purples

Rats not giving warnings:
LP's & GPs

Text output including warning for a run:

Loading file...	C:\Users\Noelle\AppData\Local\Temp\Rtmp0sDO58/af2d4d4e5ad8924fcb856730/0.mat
Stim file:	gap_40dB_1-100ms_12s
Trials: 434	Hit%: 76.4	FA%: 4.2
Analysis type: Gap (Standard)
Adding missing grouping variables: `dB`
Calculated run statistics.
Filename checks complete.
Weight checks complete.
Warning in Check_Performance_Cutoffs() : Low hit rate: 76.4%

Performance checks complete.
Archiving Run... Trials... Rat... 
Run 20230516100959_230.02199716866_2.00220159917678 of Red2 (#252) added to archives (in environment ONLY).

Warning in if (id_good() > 0) { :
  the condition has length > 1 and only the first element will be used
Run 20230516100959_230.02199716866_2.00220159917678 of Red2 (#252) saved to disk.
@nohelix nohelix added this to the keep hit percent in calc milestone May 16, 2023
@tofof
Copy link
Collaborator

tofof commented May 16, 2023

Hm, this is again duplicating our name->id conversion. Yes, the proper fix is to make sure all of the old code gets used, which again is more than simply filtering by end date - we should really throw warnings post-filter and stuff. So I think the best fix is to pull name->id completely out of process file so it's available as an independent helper function and then we can just call it directly from app.R.

@nohelix
Copy link
Collaborator Author

nohelix commented May 16, 2023

Makes sense...it will need to be accessible to main as well for auto-importing.

@tofof
Copy link
Collaborator

tofof commented May 16, 2023

Right, that's what I was saying. It will be in main but not inside process file.

@nohelix
Copy link
Collaborator Author

nohelix commented Jun 16, 2023

Hacky fix 872337c should be included in v1.0.4 as it already live.

@tofof tofof self-assigned this Jun 16, 2023
@tofof
Copy link
Collaborator

tofof commented Jun 16, 2023

Still need to do breakout to a global helper function as the proper fix, and use that everywhere. Self-assigning but don't intend to get to this urgently since the hack is adequate for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants