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

Implement slice support for sequence return types #590

Closed
mtreinish opened this issue Apr 21, 2022 · 0 comments · Fixed by #591
Closed

Implement slice support for sequence return types #590

mtreinish opened this issue Apr 21, 2022 · 0 comments · Fixed by #591
Assignees
@mtreinish
Labels
enhancement New feature or request

Comments

@mtreinish
Copy link
Member

What is the expected enhancement?

For custom sequence return types we should support slice indexing in addition to just integers. For example if you ran something like:

import retworkx
retworkx.generators.cycle_graph(24)
g = retworkx.generators.cycle_graph(24)
g.node_indices()[1:3]

It should return a NodeIndices object with the slice of 1 to 3. Right now if you want to get a slice you have to cast as a list which is less than ideal because it involves iterating and converting the entire object.
@mtreinish mtreinish added the enhancement New feature or request label Apr 21, 2022
@mtreinish mtreinish mentioned this issue Apr 21, 2022
Merged
mtreinish added a commit to mtreinish/retworkx that referenced this issue Apr 21, 2022
@mtreinish
Add support for slice inputs to custom sequence return types
a062e0b 
Fixes Qiskit#590
@mtreinish mtreinish self-assigned this Apr 21, 2022
mtreinish added a commit to mtreinish/retworkx that referenced this issue Apr 21, 2022
@mtreinish
Add support for slice inputs to custom sequence return types
293a6c7 
This commit adds support for dealing with slice inputs to custom
sequence return types such as NodeIndices. Previously, if a slice
was requested from a custom return type it would have raised a
TypeError. With this commit now it will return a new container object
with a copy of the data from the requested slice.

Fixes Qiskit#590
@mtreinish mtreinish mentioned this issue Apr 21, 2022
Merged
@mergify mergify bot closed this as completed in #591 Apr 27, 2022
mergify bot pushed a commit that referenced this issue Apr 27, 2022
@mtreinish @georgios-ts
Add support for slice inputs to custom sequence return types (#591)
7edcf9a 
* Add support for slice inputs to custom sequence return types

This commit adds support for dealing with slice inputs to custom
sequence return types such as NodeIndices. Previously, if a slice
was requested from a custom return type it would have raised a
TypeError. With this commit now it will return a new container object
with a copy of the data from the requested slice.

Fixes #590

* Fix windows build

* Fix handling of negative length int index

Previously if an integer index was passed to `__getitem__`
with a value equal to the negative length of the sequence
we would incorrectly raise an except when instead the first
element should have been returned. This commit fixes the
handling of that edge case.

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

* Fix handling of negative steps in slices

This commit fixes the handling of negative slices in sequence custom
return types. Previouly using a negative step would cause a panic
instead of correctly returning a reversed slice from the container. This
commit corrects this so we can deal with negative slice steps too.

* Apply suggestions from code review

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>

Co-authored-by: georgios-ts <45130028+georgios-ts@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mtreinish mtreinish

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for slice inputs to custom sequence return types mtreinish/retworkx
1 participant
@mtreinish