0.2.3 -> 0.3.0 shims #153
0.2.3 -> 0.3.0 shims #153
Conversation
| times = timedelta_to_float(self.data.time.values) | ||
| duration = timedelta_to_float(self.data.duration.values) | ||
|
|
||
| return np.vstack([times, times + duration]).T, list(self.data.value) |
There was a problem hiding this comment.
why np.vstack over np.array..?
There was a problem hiding this comment.
to concatenate them along the second axis; historical style holdover from the days before arbitary concat support.
but this is getting wiped in #149 also.
| @@ -104,8 +104,8 @@ def beat(ref, est, **kwargs): | |||
| namespace = 'beat' | |||
| ref = coerce_annotation(ref, namespace) | |||
| est = coerce_annotation(est, namespace) | |||
| ref_interval, _ = ref.data.to_interval_values() | |||
| est_interval, _ = est.data.to_interval_values() | |||
| ref_interval, _ = ref.to_interval_values() | |||
There was a problem hiding this comment.
ohgeez this is so much cleaner.
| intervals, values = jf.to_interval_values() | ||
| assert len(out) > 0 | ||
| assert out[0].category is DeprecationWarning | ||
| assert 'deprecated' in str(out[0].message).lower() |
| return np.vstack([times, times + duration]).T, list(self.data.value) | ||
|
|
||
| def __iter_obs__(self): | ||
| for _, (t, d, v, c) in self.data.iterrows(): |
There was a problem hiding this comment.
I find it odd that iterating self is (or will be) a shortcut to self.data.obs ... which feels like two unnecessary levels of abstraction? right?
There was a problem hiding this comment.
self.data.obs is now (will be) self.data
There was a problem hiding this comment.
... but that's neither here nor there. The point of this change is to add forward support for:
for obs in annotation:
# process jams.Observation objectthe under-the-hood parts of how that iterator works won't matter.
There was a problem hiding this comment.
I guess I meant that self -> Annotation, self.data -> AnnotationData, self.data.obs -> SortedListWithKey, and so only the last contains the items yielded by the iterator.
There was a problem hiding this comment.
ah yes, i realize I forgot to contextualize my comment -- this change is fine; however, it occurs to me that the data structure is maybe deeper than necessary? not necessarily for iteration, but mutating it?
There was a problem hiding this comment.
Right -- check the changes I made to #149 last night / this morning. That's all cleaned up now (and either way, irrelevant to this PR).
There was a problem hiding this comment.
ah yes, i see now that my understanding of things elsewhere is stale.
|
Aside from deprecation of |
|
.. I'm inclined to think there isn't much else we can do here, since any other deprecation warnings would apply to methods called internally as well as the public. Unless there are objections from @ejhumphrey , I think this is ready to merge and push as 0.2.3 release. |
|
i have no other thoughts or opinions at the moment, tho I apologize in advance for when they occur to me in 4 days. |
This PR adds deprecation warnings and some forward-looking compatibility for #149 .
Implements #152.