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

Added correct indexing for negative steps and check for 0-index #263

Closed
wants to merge 1 commit into from

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented Jun 29, 2015

This fixes stuff like
a = {1,2,3,4,5}
a[[3;;1;;-1]] == {3,2,1}

which previously resulted in {2}.

This should fix #196

@sn6uv sn6uv added this to the 0.9 milestone Jun 29, 2015
@sn6uv
Copy link
Member

sn6uv commented Jul 5, 2015

Thanks for fixing this. I'm finding the python_seq function a little bit confusing. Do you think you could clean it up and add a short docstring explaining its purpose?

@sn6uv sn6uv removed the needs review label Jul 6, 2015
@wolfv
Copy link
Member Author

wolfv commented Jul 7, 2015

Do you think there is a way to fix all of these at once?

Ie. Table[i,{i,1,-10,-1}] -> {1,0,-1...-10} but evaluates to {} currently
Also Range[i+1, i+10] -> {i+1, i+2, i+3...} but doesn't evaluate at all.
Range[0,-10,-1] doesnt work either.

@wolfv
Copy link
Member Author

wolfv commented Jul 10, 2015

def python_seq(start, stop, step=+1):
    # step can be None, which means positive stepsize
    if not step: step = +1

    # What this function does is convert a 1 based index to
    # zero based index. Additionally, the start of a list
    # is indicated by None in python.
    if step > 0:
        if start > 0:
            start -= 1
        if stop is not None and stop < 0:
            stop += 1
            if stop == 0:
                stop = None
        return start, stop
    else:
        # in the case of a negative step, 
        # admissible values are start < stop
        # so that start get's decremented until stop is 
        # reached. 
        # However, note that in python a = [1, 2, 3, 4][3:0:-1] == [4, 3, 2]
        # and not, as one might think [4, 3, 2, 1] (which is the result of [3:None:-1])
        # Therefore, stop has -2 subtracted, and None is returned when stop == -1
        if stop and stop > 0:
            stop -= 2

        elif stop and stop < 0:
            stop -= 1

        if start and start > 0:
            # zero base index
            start -= 1

        if not start:
            # no start index = include all (0)
            start = None

        if stop == -1:
            # stop equals end, which equals -1
            stop = None
        elif stop == None:
            # No stop equals len(list)
            # This is actually weird in Mathematica. 
            # E.g. a = {1,2,3,4}; a[[4;; ;; -1]] == {4}
            # but all other combinations evaluate to an error
            # a[[3 ;;  ;; -1]] = cannot take positions -4 through -1
            # in Mathics it all evaluates to an empty list currently.
            stop = -1
        return start, stop

@sn6uv
Copy link
Member

sn6uv commented Sep 25, 2015

What's the status of this PR? That code in the comments looks much better but there are still some unhandled cases I think e.g. python_seq(-2, -3, 1) returns (-2, -2).

@wolfv
Copy link
Member Author

wolfv commented Sep 25, 2015

Your example is the actually correct:

selection_081

But you're right that this piece of code should get some proper testing and specification.

@sn6uv sn6uv mentioned this pull request Feb 14, 2016
4 tasks
@sn6uv
Copy link
Member

sn6uv commented Feb 14, 2016

I'm planning to release Mathics 0.9 in the next couple of weeks. Would love to get these changes merged in if possible. @wolfv are you planning to finish this?

sn6uv added a commit to sn6uv/Mathics that referenced this pull request Feb 22, 2016
@sn6uv sn6uv mentioned this pull request Feb 22, 2016
@sn6uv
Copy link
Member

sn6uv commented Feb 22, 2016

closed in favour of #325

@sn6uv sn6uv closed this Feb 22, 2016
sn6uv added a commit to sn6uv/Mathics that referenced this pull request Feb 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Span does not behave correctly
2 participants