Skip to content

Add Digitization class#6362

Closed
massich wants to merge 18 commits intomne-tools:masterfrom
massich:refactor_digitation
Closed

Add Digitization class#6362
massich wants to merge 18 commits intomne-tools:masterfrom
massich:refactor_digitation

Conversation

@massich
Copy link
Copy Markdown
Contributor

@massich massich commented May 22, 2019

This PR:

  • takes over [WIP] refactor to prepare digitization #5986.
  • modifies _test_raw_reader to force info['dig'] to be Digitization or None
  • Add Digitization class
  • Refactors part of the digitization to make them look similar (for further refactor).

On the top, here is a summary of which readers go through _test_raw_reader, if so with was type(info['dig']): None, Digitization, or both. And for those readers expected to not have Digitization do they have montage. The goal of this table is to keep in mind the current status + plan following PRs.

                       | _test_raw_reader | None | Digitization | other   |
-----------------------+------------------+------+--------------+---------+
RawArray               |     x            |  x   |              |         |
read_raw_artemis123    |     x            |      |     x        |         |   
read_raw_bdf           |                  |      |              | montage |
read_raw_brainvision   |     x            |  x   |     x        |         |    
read_raw_bti           |     x            |  x   |     x        |         |
read_raw_cnt           |     x            |  x   |              | montage |
read_raw_ctf           |     x            |      |     x        |         |
read_raw_edf           |     x            |  x   |     x        |         |
read_raw_eeglab        |     x            |      |     x        |         |
read_raw_egi           |     x            |  x   |              | montage |
read_raw_eximia        |     x            |  x   |              | ------- |
read_raw_fieldtrip     |                  |      |              | ------- |
read_raw_fif           |     x            |      |     x        |         |
read_raw_gdf           |     x            |  x   |              | montage |
read_raw_kit           |     x            |  x   |     x        |         |
read_raw_nicolet       |     x            |  x   |              | montage |

This PR should be green. There are still some funky names _foo, _bar. I will soon push a commit to amend that. But at least it would be easier to read, give comments (and if someone has strong suggestions for the funky names, now is the time)

cc: @teonbrooks, @larsoner

Joan Massich and others added 15 commits May 22, 2019 16:28
move Digpoint to digitization
368ffed
ENH: Add Digitization
1be3601
ENH: allow Digitization comparison
1dd80da
Create a helper class for MNE objects list
92e1f3f
@teonbrooks
move kit stuff
d87be64 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
@teonbrooks
fix: kitEpoch.__init__
28154aa 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
@teonbrooks
migrate artemis123
dd7d21c 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
@teonbrooks
start moving BTI (after BTI refactor)
4af597d 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
update the signature to use nasion, lpa, rpa, hpi, extra
0472dd6
@teonbrooks
unpack idx_points inside _read_head_file
8e5e4ef 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
@teonbrooks
ENH: Add Digitization class
eb4a71a 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
@teonbrooks
MNT: split _makedig_points
237fc49 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
remove redundant operation
fa1c20f
@teonbrooks
Fix: __delitem__
51ea4dc 
Co-authored-by: Teon Brooks <teon.brooks@gmail.com>
pep
0c94c97
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 22, 2019

This pull request introduces 1 alert when merging 0c94c97 into b9b2c68 - view on LGTM.com

new alerts:

  • 1 for Unreachable code

Comment posted by LGTM.com

teonbrooks
teonbrooks reviewed May 22, 2019
View reviewed changes
mne/digitization/_mess.py Outdated
@@ -0,0 +1,153 @@
"""Do not look at this !! or blame Teon !!!."""
Copy link
Copy Markdown
Member

@teonbrooks teonbrooks May 22, 2019

Choose a reason for hiding this comment

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

😂😂😂

@codecov
Copy link
Copy Markdown

codecov bot commented May 23, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@b9b2c68). Click here to learn what that means.
The diff coverage is 33.91%.

@@            Coverage Diff            @@
##             master    #6362   +/-   ##
=========================================
  Coverage          ?   15.84%           
=========================================
  Files             ?      410           
  Lines             ?    74372           
  Branches          ?    12300           
=========================================
  Hits              ?    11784           
  Misses            ?    62275           
  Partials          ?      313

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented May 23, 2019

This pull request introduces 1 alert when merging 73bd194 into b9b2c68 - view on LGTM.com

new alerts:

  • 1 for Unreachable code

Comment posted by LGTM.com

@massich
Copy link
Copy Markdown
Contributor Author

massich commented May 23, 2019

From conversation with @agramfort IRL, I'll split this PR in two.

@massich massich force-pushed the refactor_digitation branch from 6124bc3 to 1a45b31 Compare May 23, 2019 11:44
@massich massich closed this May 23, 2019
@massich massich mentioned this pull request May 23, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@teonbrooks teonbrooks teonbrooks left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@massich @teonbrooks