STY: prefer _DefaultEventParser to _Counter#6355
Merged
massich merged 2 commits intomne-tools:masterfrom May 22, 2019
Merged
Conversation
Member
|
why is the size of raw a problem? it's passed by reference. What do I miss. Patch looks good |
Contributor
Author
|
It's just me getting confused. https://docs.python.org/3/faq/programming.html#how-do-i-write-a-function-with-output-parameters-call-by-reference |
Codecov Report
@@ Coverage Diff @@
## master #6355 +/- ##
==========================================
+ Coverage 89.31% 89.31% +<.01%
==========================================
Files 405 405
Lines 74266 74271 +5
Branches 12297 12297
==========================================
+ Hits 66330 66335 +5
Misses 5072 5072
Partials 2864 2864 |
Contributor
Author
|
This would make codecov more happy, but I don't think is more readable. def __call__(self, description, offset=1):
if description not in self.event_ids:
out = offset + len(self.event_ids)
self.event_ids[description] = out
else:
out = self.event_ids[description]
return out( |
Member
is equiv to |
Contributor
Author
~/code/mne-python refactor_digitation* 2m 55s
(mne) ❯ ipython
Python 3.6.8 |Anaconda, Inc.| (default, Dec 30 2018, 01:22:34)
Type 'copyright', 'credits' or 'license' for more information
IPython 7.4.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: %paste
def xx():
dd = dict()
for _ in range(50):
if 'a' not in dd:
dd['a'] = len(dd)+1
out = dd['a']
for _ in range(50):
dd = dict()
if 'a' not in dd:
dd['a'] = len(dd)+1
out = dd['a']
def yy():
dd = dict()
for _ in range(50):
dd['a'] = dd.get('a', len(dd)+1)
out = dd['a']
for _ in range(50):
dd['a'] = dd.get('a', len(dd)+1)
out = dd['a']
## -- End pasted text --
In [2]: %timeit -n100 -r100 xx()
11.5 µs ± 2.31 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [3]: %timeit -n100 -r100 yy()
14.9 µs ± 2.42 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [4]: %timeit -n100 -r100 xx()
10.6 µs ± 701 ns per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [5]: %timeit -n100 -r100 yy()
The slowest run took 7.67 times longer than the fastest. This could mean that an intermediate result is being cached.
17 µs ± 13 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [6]: %timeit -n100 -r100 xx()
11.2 µs ± 1.74 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [7]: %timeit -n100 -r100 yy()
14.9 µs ± 3.33 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [8]: %timeit -n100 -r100 xx()
11.3 µs ± 1.58 µs per loop (mean ± std. dev. of 100 runs, 100 loops each)
In [9]: %timeit -n100 -r100 yy()
14.5 µs ± 1.57 µs per loop (mean ± std. dev. of 100 runs, 100 loops each) |
Contributor
Author
|
I'll merge as it is. Thx @larsoner |
larsoner
pushed a commit
to larsoner/mne-python
that referenced
this pull request
Jun 11, 2019
agramfort
pushed a commit
that referenced
this pull request
Jun 12, 2019
* FIX: allow raw_fif to parse brainvision events (#6417) * TST: usecase * FIX: check annotation description base names * fix: remove pathlib * RFC: Alternative without getattr. * imports * Fix: avoid adding 'barinvision' as event_id keyword * remove _get_auto_event_id * update whatsnew * Update brainvision.py * cosmit * fix * use rstrip * MRG+1, DOC: footer logos (#6420) * add individual inst. logos; add uni-graz * add image titles (hover) * force add missing logos * codespell skip SVG files * fix: make inria PNG; better logo wrapping * fix footer overlap; slightly larger logos * STY: prefer _DefaultEventParser to _Counter (#6355) * Fix docstrings (#6385) * docstring deduplication * more n_jobs * fix: import fill_doc * fix crossrefs; deduplicate docdict def for random_state * FIX: Conftest [circle front] * FIX: Ref [circle front]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Things I wanted to ask in #6326 but I didn't have the chance.
It avoids returning
lambda: None+ adds a test in caseevent_id='random string'. What I did not got away with is a form to not passrawto_check_event_id. (which can be big and so on). @larsoner can you see a way? Maybe defining the function within or braking it again in two functions which we end up at the same place as we started.Anyways, feel free to push in the branch or close