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

[BUG] Support for negative index in the List._reverse() method #2496

Closed
msaelices opened this issue May 3, 2024 · 4 comments
Closed

[BUG] Support for negative index in the List._reverse() method #2496

msaelices opened this issue May 3, 2024 · 4 comments
Labels
bug Something isn't working mojo-repo Tag all issues with this label mojo-stdlib Tag for issues related to standard library q2-2024

Comments

@msaelices
Copy link

msaelices commented May 3, 2024 •
edited by ematejska

Bug description

Reviewing the collections.List struct, I've found that the _reverse(start) private method is not supporting negative indexes

Steps to reproduce

This piece of code will not work as expected:

    vec = List[Int]()
    vec.append(1)
    vec.append(2)
    vec.append(3)
 
    vec._reverse(start=-2)

System information

- What OS did you do install Mojo on ?

Ubuntu 22.04.4 LTS
  • Provide version information for Mojo by pasting the output of mojo -v
mojo 2024.5.303 (0b4c4e9b)
  • Provide Modular CLI version by pasting the output of modular -v
modular 0.7.4 (df7a9e8b)
@msaelices msaelices added bug Something isn't working mojo-repo Tag all issues with this label labels May 3, 2024
@msaelices msaelices mentioned this issue May 3, 2024
Closed
@JoeLoser JoeLoser added the mojo-stdlib Tag for issues related to standard library label May 4, 2024
@vidur2
Copy link

vidur2 commented May 4, 2024

Can I take a stab at this?

@JoeLoser
Copy link
Collaborator

JoeLoser commented May 4, 2024

Can I take a stab at this?

Of course, go for it! Happy to review a PR.

@soraros
Copy link
Contributor

soraros commented May 4, 2024

@vidur2 I think @msaelices already opened #2497 for this.

@vidur2
Copy link

vidur2 commented May 4, 2024

Oh ok thank you

@msaelices msaelices mentioned this issue May 5, 2024
Closed
@ematejska ematejska removed the mojo-stdlib Tag for issues related to standard library label May 6, 2024
@ematejska ematejska added the mojo-stdlib Tag for issues related to standard library label May 7, 2024 — with Linear
modularbot pushed a commit that referenced this issue May 16, 2024
@msaelices @ConnorGray @modularbot
[External] [stdlib] Support for negative indexes in `List._reverse(st…
02aa519 
…art)` method and out of bound validation (#39464)

[External] [stdlib] Support for negative indexes in
`List._reverse(start)` method and out of bound validation

## Changes

* The `_reverse(start)` private method in the `collections.List` struct
does not support negative indexes.
* Check out-of-bound indexes in the `List._reverse()` method
* Tests

## Ref

Fixes #2496

---------

Co-authored-by: Manuel Saelices <[email protected]>
Co-authored-by: Connor Gray <[email protected]>
Closes #2520
MODULAR_ORIG_COMMIT_REV_ID: 2662cfba2e6c1a602e8fa11880f29f8add446506
@rparolin rparolin added the q2-2024 label May 16, 2024 — with Linear
lsh pushed a commit to lsh/mojo that referenced this issue May 17, 2024
@msaelices @lsh
[External] [stdlib] Support for negative indexes in `List._reverse(st…
2089c38 
…art)` method and out of bound validation (#39464)

[External] [stdlib] Support for negative indexes in
`List._reverse(start)` method and out of bound validation

## Changes

* The `_reverse(start)` private method in the `collections.List` struct
does not support negative indexes.
* Check out-of-bound indexes in the `List._reverse()` method
* Tests

## Ref

Fixes modularml#2496

---------

Co-authored-by: Manuel Saelices <[email protected]>
Co-authored-by: Connor Gray <[email protected]>
Closes modularml#2520
MODULAR_ORIG_COMMIT_REV_ID: 2662cfba2e6c1a602e8fa11880f29f8add446506

Signed-off-by: Lukas Hermann <[email protected]>
msaelices added a commit to msaelices/mojo that referenced this issue May 18, 2024
@msaelices @ConnorGray
[External] [stdlib] Support for negative indexes in `List._reverse(st…
7f67f38 
…art)` method and out of bound validation (#39464)

[External] [stdlib] Support for negative indexes in
`List._reverse(start)` method and out of bound validation

## Changes

* The `_reverse(start)` private method in the `collections.List` struct
does not support negative indexes.
* Check out-of-bound indexes in the `List._reverse()` method
* Tests

## Ref

Fixes modularml#2496

---------

Co-authored-by: Manuel Saelices <[email protected]>
Co-authored-by: Connor Gray <[email protected]>
Closes modularml#2520
MODULAR_ORIG_COMMIT_REV_ID: 2662cfba2e6c1a602e8fa11880f29f8add446506
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working mojo-repo Tag all issues with this label mojo-stdlib Tag for issues related to standard library q2-2024
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@msaelices @rparolin @soraros @JoeLoser @ematejska @vidur2