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.

Sign up for GitHub

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

Jump to bottom

adding greedy sensor manager #874

Merged
merged 5 commits into from
Oct 25, 2023
Merged

adding greedy sensor manager #874

merged 5 commits into from
Oct 25, 2023

Conversation

akenyon
Copy link
Contributor

@akenyon akenyon commented Oct 10, 2023

No description provided.

@akenyon akenyon requested a review from a team as a code owner October 10, 2023 14:29
@akenyon akenyon requested review from orosoman-dstl, mharris-dstl, nperree-dstl and spike-dstl and removed request for a team, orosoman-dstl and mharris-dstl October 10, 2023 14:29
spike-dstl
spike-dstl reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@spike-dstl spike-dstl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Was able to get an example running using GreedySensorManager that gave results comparable to BruteForceSensorManager. Left a couple picky comments that could be changed.

stonesoup/sensormanager/tests/test_sensormanager.py Outdated Show resolved Hide resolved
stonesoup/sensormanager/base.py Outdated Show resolved Hide resolved
nperree-dstl
nperree-dstl approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@nperree-dstl nperree-dstl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. We should probably use a conftest.py file for the other sensor manager tests too but can change that separately.

@sdhiscocks sdhiscocks merged commit 0669873 into main Oct 25, 2023
@sdhiscocks sdhiscocks deleted the greedy2 branch October 25, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdhiscocks sdhiscocks sdhiscocks left review comments

@spike-dstl spike-dstl spike-dstl approved these changes

@nperree-dstl nperree-dstl nperree-dstl approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@akenyon @sdhiscocks @nperree-dstl @spike-dstl