ENH: add Epoch.__len__

[christianbrodbeck]

Consistent with indexing and iteration behavior.

[larsoner]

LGTM

[dengemann]

@christianmbrodbeck I proposed this one a few months ago and before me someone else did. Both times -1 ...

[dengemann]

please consider #253

[christianbrodbeck]

@dengemann What is the argument against it? Or do you remember the specific PRs?

[dengemann]

see #253 --- bad experiences with dropped bads --- IIRC @agramfort and @mluessi had this before and removed it ...

[christianbrodbeck]

Ok, bad-dropped. That's why I added the warning, to be consistent with indexing behavior (which indexes the events and issues a warning). If you think that length should be more restrictive it could raise an error if bads have not been dropped? I still think it would be useful when working with preload=True.

[larsoner]

Throwing an error sounds like the safest behavior to me. We'll see what @agramfort and @mluessi think about it.

[dengemann]

taking the devil's advocate's role: what's a len method that only works conditionally. In my scripts I quite often change preload flags, so I would never use this as it would imply unnecessary conditionals ...

[larsoner]

Yeah, I think that would be a feature and not a bug, in the sense that you wouldn't want the len() method to work unless epochs have bad events dropped. Thus it would be a feature for people who always preload epochs or drop bad epochs from them (I am one of these people) while being protected against people using it incorrectly. That being said, I'm not sure I'd use the feature for things I do now, but who knows in the future.

[dengemann]

... continuing: then this is YGNI?

[larsoner]

Well, I maybe ain't gonna need it, but apparently @christianmbrodbeck has a use case...?

[dengemann]

Please don't get me wrong -- I would like it. Trying to accumulate pros

[agramfort]

I am still not convinced due the conditioning issue raised by @dengemann

Should we vote for it, i vote for an exception more than a warning of inaccurate result.

Also there is no test.

My vote is -0.5 :)

[larsoner]

My vote is 0.0 (indifference). If abstention isn't allowed, I'll go with +0.5. I agree that it should be an exception instead of a warning if they try to get len() without bads dropped. @mluessi what do you think? If your opposition is not strong, @dengemann and @christianmbrodbeck's votes for might win the day...

[dengemann]

Yeah, thinking about it for a few days, subcortically, unconsciously, etc., I'm not opposed to it. If we add an exception that should be fine. In a certain sense that would be pythonic since getting exceptions from invoking len can also happen in the world of builtin types. + 0.5

[agramfort]

ok fair enough. Let me know when it's an exception so we can merge.

[christianbrodbeck]

Made it an exception and added tests.

[agramfort]

thanks !

[mluessi]

PS: I was +1 for merge as long as it throws an exception as well.