Track metadata handling update #121
Merged
Conversation
This file contains 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
sglvladi
commented
Jul 10, 2019
| # from all hypotheses are retained, but more likely | ||
| # hypotheses will over-write the metadata set by less likely | ||
| # ones. | ||
| for hypothesis in sorted(state.hypothesis, reverse=True): |
There was a problem hiding this comment.
Not sure if this is correct. My (possibly wrong) intuition says that calling sorted on a MultipleHypothesis object should return a list of hypotheses, with the most likely being first. In the case of SingleProbabilityHypothesis this means that the hypothesis with highest probability would come first, while for SingleDistanceHypothesis the hypothesis with lowest distance would be first.. Thus, to have them sorted in reverse, I have included the argument reverse=True.
sdhiscocks
approved these changes
Aug 19, 2019
| @@ -61,6 +61,9 @@ class StateMutableSequence(Type, MutableSequence): | |||
| def __init__(self, states=None, *args, **kwargs): | |||
| if states is None: | |||
| states = [] | |||
| elif not isinstance(states, list): | |||
There was a problem hiding this comment.
May be better to check if states is a collections.abc.MutableSequence instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Following discussions with @sdhiscocks during Fusion in relation to #118, this pull request aims to solve the performance issues relating to reading metadata as follows:
Track.metadata()setter method).As part of the above, a small correction has also been applied to
StateMutableSequenceto ensure that thestatesproperty is always a list after initialisation, even if a singleStateobject is passed. Previously, a statement of the formtrack = Track(states=State(np.array([[1]]), timestamp=datetime.datetime.now())would causetrack.statesto be aStateobject, rather than a list of length 1.