lkorczowski / Tinnitus-n-Sleep

Detecting events in sleeping tinnitus patients
MIT License
1 stars 0 forks source link

completing reports for laterality #110

Closed RobinGuillard closed 3 years ago

RobinGuillard commented 3 years ago
review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov[bot] commented 3 years ago

Codecov Report

Merging #110 (083fb5a) into master (5ded0a1) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #110   +/-   ##
=======================================
  Coverage   99.26%   99.26%           
=======================================
  Files          11       11           
  Lines         816      816           
=======================================
  Hits          810      810           
  Misses          6        6           

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 5ded0a1...083fb5a. Read the comment docs.

review-notebook-app[bot] commented 3 years ago

View / edit / reply to this conversation on ReviewNB

lkorczowski commented on 2020-11-24T12:53:33Z ----------------------------------------------------------------

for filename in results.keys():
   if filename[-4:]=="left":
       if filename!="1SL21_nuit_3.edf_left":
           if filename!='1CB23_cohort2.edf_left':
               if filename!='1HB20_nuit_2.edf_left':
                   if filename!='1UC22_nuit_2.edf_left':
                       if filename!='3KB06_cohort2.edf_left':
                           if filename!='2TL07_nuit_1.edf_left':

                               laterality_study.append(filename[:-5])

Is it magic ? avoid at any cost. Use data_info if needed.


review-notebook-app[bot] commented 3 years ago

View / edit / reply to this conversation on ReviewNB

lkorczowski commented on 2020-11-24T12:53:34Z ----------------------------------------------------------------

maybe there is a way to make the report more readible ? e.g. print a DataFrame ?


review-notebook-app[bot] commented 3 years ago

View / edit / reply to this conversation on ReviewNB

lkorczowski commented on 2020-11-24T12:58:14Z ----------------------------------------------------------------

could you make the output more readible ? e.g. use dataframe the same way I'm using it in Inter_subjects NB ?

Also, avoid hardcoding subject rejection. We have data_info for that.


review-notebook-app[bot] commented 3 years ago

View / edit / reply to this conversation on ReviewNB

lkorczowski commented on 2020-11-24T12:58:15Z ----------------------------------------------------------------

use DataFrame ?


review-notebook-app[bot] commented 3 years ago

View / edit / reply to this conversation on ReviewNB

lkorczowski commented on 2020-11-24T12:58:16Z ----------------------------------------------------------------

why this code is duplicated ?


lkorczowski commented 3 years ago

@RobinGuillard merge when you want but please read my recommendations for good practices

  • not using hard-coded variables in NB such as users (use instead data_info.csv which has the advantage for being shared across NB).
  • use readable output in NB