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

Rename of getThreadIndex to getIndex #943

Merged
merged 4 commits into from
Oct 26, 2022
Merged

Rename of getThreadIndex to getIndex #943

merged 4 commits into from
Oct 26, 2022

Conversation

mondus
Copy link
Member

@mondus mondus commented Oct 24, 2022

Fixes #898 .

Simple fix by renaming. Not used in any tests only internally in a few places. Testing of this seems like overkill.

Addition of getIndex to host API can be dealt with as a separate enhancement issue as it is already achievable using iterator logic.

@ptheywood
Copy link
Member

ptheywood commented Oct 24, 2022

This does not close the referenced issue, which is explicilty for a getIndex method which is for both the hsot and device components, although it is part of that issue (and the most immediately required one.

Could you edit the PR description and remove the Fixes statement so it doens't auto close the issue? We can move it in the prioritisaiton board when this is done.

@mondus
Copy link
Member Author

mondus commented Oct 24, 2022

This does not close the referenced issue, which is explicilty for a getIndex method which is for both the hsot and device components, although it is part of that issue (and the most immediately required one.

Could you edit the PR description and remove the Fixes statement so it doens't auto close the issue? We can move it in the prioritisaiton board when this is done.

@ptheywood I've just added the host version of this instead along with a test for host side which is less trivial than the device API.

Copy link
Member

@ptheywood ptheywood left a comment

Choose a reason for hiding this comment

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

Added a python test for AgentVector for you, but not for DeviceAgentVector as there isn't a test file for that yet (see #557).

Otherwise looks good to (squash) merge (subject to CI)

@ptheywood ptheywood merged commit e8b23b2 into master Oct 26, 2022
@ptheywood ptheywood deleted the get_index_func branch October 26, 2022 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Host/Device API agent getIndex method
2 participants