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

brainvision tests #2

Merged
merged 4 commits into from
Aug 2, 2019
Merged

brainvision tests #2

merged 4 commits into from
Aug 2, 2019

Conversation

massich
Copy link

@massich massich commented Aug 2, 2019

No description provided.

@massich
Copy link
Author

massich commented Aug 2, 2019

That's how I would have written it. But I miss understood the yield. (I think)

@massich
Copy link
Author

massich commented Aug 2, 2019

cf69e87 should make the trick. And we get nice naming:

~/code/mne-python bvtests* 12s
(mne) ❯ pytest ./mne/io/brainvision/tests/test_brainvision.py -k foo -vs      
Coverage.py warning: --include is ignored because --source is set (include-ignored)
Test session starts (platform: linux, Python 3.6.8, pytest 4.4.0, pytest-sugar 0.9.2)
cachedir: .pytest_cache
rootdir: /home/sik/code/mne-python, inifile: setup.cfg
plugins: timeout-1.3.3, sugar-0.9.2, pudb-0.7.0, mock-1.10.3, faulthandler-1.5.0, cov-2.6.1
collecting ... ('Mk1=New Segment,,1,1,0,20131113161403794232\n', list([1384359243, 794231]), '2013-11-13 16:14:03 GM')

 mne/io/brainvision/tests/test_brainvision.py::test_foo[foo0] ✓                                                                                                                                                          25% ██▌       ('Mk1=STATUS,,1,1,0\n', None, 'unspecified')

 mne/io/brainvision/tests/test_brainvision.py::test_foo[foo1] ✓                                                                                                                                                          50% █████     ('Mk1=New Segment,,1,1,0,\n', None, 'unspecified')

 mne/io/brainvision/tests/test_brainvision.py::test_foo[foo2] ✓                                                                                                                                                          75% ███████▌  ('Mk1=New Segment,,1,1,0\n', None, 'unspecified')

 mne/io/brainvision/tests/test_brainvision.py::test_foo[foo3] ✓                                                                                                                                                         100% ██████████
----------------------------------------------------------------------------------- generated xml file: /home/sik/code/mne-python/junit-results.xml -----------------------------------------------------------------------------------


====================================================================================================== slowest 20 test durations ======================================================================================================
1.59s setup    mne/io/brainvision/tests/test_brainvision.py::test_foo[foo0]

(0.00 durations hidden.  Use -vv to show these durations.)

Results (10.80s):
       4 passed
      16 deselected

~/code/mne-python bvtests* 11s
(mne) ❯ 

Copy link
Author

@massich massich left a comment

Choose a reason for hiding this comment

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

Maybe we could avoid copying all the files if we were just testing the inner function that recovers the info from the vmrk. But maybe we wont have a full info field.

# ('Mk1=New Segment,,1,1,0\n', 'unspecified'),
# ('Mk1=New Segment,,1,1,0,00000000000304125000', 'unspecified'),
# ('Mk1=New Segment,,1,1,0,\nMk2=New Segment,,2,1,0,20070716122240937454\n', '2007-07-16 12:22:40 GMT'), # noqa: E501
# ])
Copy link
Author

Choose a reason for hiding this comment

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

@sappelhoff can you finish to add the missing tests?

Copy link
Owner

Choose a reason for hiding this comment

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

yes +1

@pytest.fixture(scope='session')
def _mocked_meas_date_data(tmpdir_factory):
"""Helper funct. preparing the files for mocked_meas_date_file fixture."""

Copy link
Author

Choose a reason for hiding this comment

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

maybe we don't need this empty line

Copy link
Owner

Choose a reason for hiding this comment

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

no, I'll delete it

vhdr_fname = _mocked_meas_date_data['vhdr_fname']
vmrk_fname = _mocked_meas_date_data['vmrk_fname']
eeg_fname = _mocked_meas_date_data['eeg_fname']
lines = _mocked_meas_date_data['lines']
Copy link
Author

Choose a reason for hiding this comment

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

maybe there is a nicer way to unpack the dictionary. I started doing it with a for loop but it was looking even more ugly.

Copy link
Owner

Choose a reason for hiding this comment

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

We could use an OrderedDict and use

vhdr_fname, vmrk_fname , eeg_fname, lines = _mocked_meas_date_data.values()

also: we don't need eeg_fname, so I'll cut that

vmrk_fname=vmrk_fname,
eeg_fname=eeg_fname,
expected_meas_date=request.param['meas_date'],
expected_meas_date_repr=request.param['meas_date_repr'])
Copy link
Author

Choose a reason for hiding this comment

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

we only need vhrd_fname. If so:

    yield dict(fname=vhdr_fname,
               expected_meas_date=request.param['meas_date'],
               expected_meas_date_repr=request.param['meas_date_repr'])

Copy link
Owner

Choose a reason for hiding this comment

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

yes, I'll change that to the reduced version 👍

Copy link
Owner

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Thx!

# ('Mk1=New Segment,,1,1,0\n', 'unspecified'),
# ('Mk1=New Segment,,1,1,0,00000000000304125000', 'unspecified'),
# ('Mk1=New Segment,,1,1,0,\nMk2=New Segment,,2,1,0,20070716122240937454\n', '2007-07-16 12:22:40 GMT'), # noqa: E501
# ])
Copy link
Owner

Choose a reason for hiding this comment

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

yes +1

@pytest.fixture(scope='session')
def _mocked_meas_date_data(tmpdir_factory):
"""Helper funct. preparing the files for mocked_meas_date_file fixture."""

Copy link
Owner

Choose a reason for hiding this comment

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

no, I'll delete it

vhdr_fname = _mocked_meas_date_data['vhdr_fname']
vmrk_fname = _mocked_meas_date_data['vmrk_fname']
eeg_fname = _mocked_meas_date_data['eeg_fname']
lines = _mocked_meas_date_data['lines']
Copy link
Owner

Choose a reason for hiding this comment

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

We could use an OrderedDict and use

vhdr_fname, vmrk_fname , eeg_fname, lines = _mocked_meas_date_data.values()

also: we don't need eeg_fname, so I'll cut that

vmrk_fname=vmrk_fname,
eeg_fname=eeg_fname,
expected_meas_date=request.param['meas_date'],
expected_meas_date_repr=request.param['meas_date_repr'])
Copy link
Owner

Choose a reason for hiding this comment

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

yes, I'll change that to the reduced version 👍

@sappelhoff sappelhoff merged commit ee6544f into sappelhoff:bvtests Aug 2, 2019
@massich massich deleted the bvtests branch August 3, 2019 11:17
@massich massich restored the bvtests branch August 3, 2019 11:18
@massich massich deleted the bvtests branch August 5, 2019 15:40
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.

2 participants