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

Trackers are Iterables instead of Generators #415

Merged
merged 1 commit into from
Feb 25, 2022
Merged

Conversation

gawebb-dstl
Copy link
Contributor

@gawebb-dstl gawebb-dstl commented Mar 18, 2021

The trackers have been turned into iterables instead of generators. This allows trackers to be copied/ manipulated while producing tracks if needed.

Instead of for time, tracks in tracker.tracks_gen():
you now write
for time, tracks in tracker:

There should be no other changes to how the trackers function

@gawebb-dstl
Copy link
Contributor Author

@sdhiscocks I've added a generator method (with a deprecation warning) into the base tracker so that the tracker will function as normal. This is so people's code doesn't (hopefully) break straight away

@sdhiscocks
Copy link
Member

If we are going to make a change, we should probably extend this to all other classes that use the BufferedGenerator: Reader, Feeder, Detector, Simulator and Writer.

I think migrating to making these classes iterators (and thinking iterator rather than iterable to be clear on distinction) is good idea. This is more standard approach than using BufferedGenerator, which was kinda working around the fact the class instances were not iterable in the first place.

On key element of iterators, is subsequent calls to __iter__ shouldn't reset or modify the calls to __next__. I don't think this should be an issue, but may need to see when code changes. Note that one of readers that I made a change to in this PR breaks that, but should be easy to resolve.

@gawebb-dstl gawebb-dstl marked this pull request as ready for review February 24, 2022 17:35
@gawebb-dstl gawebb-dstl requested a review from a team as a code owner February 24, 2022 17:35
@gawebb-dstl gawebb-dstl requested review from sdhiscocks and svidal-dstl and removed request for a team February 24, 2022 17:35
This removes the tracks_gen() method on Tracker classes, but instances
are iterable the same as BufferedGenerator version.
@sdhiscocks sdhiscocks added breaking A breaking change that required other to modify their code and removed work in progress discussion labels Feb 25, 2022
@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #415 (5358490) into main (071abc8) will decrease coverage by 0.02%.
The diff coverage is 99.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #415      +/-   ##
==========================================
- Coverage   94.37%   94.34%   -0.03%     
==========================================
  Files         154      154              
  Lines        7443     7463      +20     
  Branches     1414     1409       -5     
==========================================
+ Hits         7024     7041      +17     
- Misses        315      317       +2     
- Partials      104      105       +1     
Flag Coverage Δ
integration 67.96% <48.18%> (-0.03%) ⬇️
unittests 91.73% <99.09%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
stonesoup/tracker/simple.py 99.09% <98.66%> (+0.13%) ⬆️
stonesoup/reader/yaml.py 95.23% <100.00%> (+0.41%) ⬆️
stonesoup/tracker/base.py 100.00% <100.00%> (ø)
stonesoup/tracker/pointprocess.py 93.54% <100.00%> (-4.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 071abc8...5358490. Read the comment docs.

@sdhiscocks sdhiscocks merged commit d648229 into main Feb 25, 2022
@sdhiscocks sdhiscocks deleted the iterable_trackers branch February 25, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change that required other to modify their code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants