Command to store from stack into variable

[nfraprado]

I'm not sure this is something that makes part of the RPN workflow (I admit to not having used RPN much before), but I find myself wanting to save the value that is on top of the stack to a variable for later use, but from what I saw it seems this is not currently possible.

To me the most convenient/intuitive syntax would be to input my_var=, and with that, the value on top of the stack would be stored in the my_var variable, but it isn't much like the rest of rpnpy's syntax, so perhaps it would complicate the parsing code too much...

In any case, I think at least what could be done is to input the variable name as a string in the stack: 'my_var', and then use a special command store, that accepts two parameters, the first is the value and the second, the variable name. After that the value would be accessible in the my_var variable as usual.

While we're at it, might as well make the store special command accept a list, so that it is possible to save the whole stack with 'my_var', and then store:*. Then, with the stack stored as a list in the my_var variable, one could later restore the stack (assuming the stack is now empty) with my_var:i.

What do you think, @terrycojones ? Is this a reasonable idea at all? Is some of this already available in rpnpy? Any more thoughts?

[terrycojones]

Hello again!

I think this makes good sense. In dc there is an s (store) command and an l (load) command. You can use s to save the value on the top of the stack into a variable (a through z). The value is taken off the stack.

I think it would make sense to have a special store command, as you suggest. with behaviour:

• If it is given no argument, it expects to find the name of the variable on the top of the stack (as you suggest).
• But it can be given an optional variable name, in which case the value(s) are stored into that variable.

store could follow the convention of acting on just the top stack element unless you said :3 or :* in which case a list would be stored.

I'm not sure if the value(s) should then be popped off the stack, but I think they should (which matches dc). The user could use the regular = modifier to leave the stack as it was (in which case the value would still be in the variable).

Something like that?

[nfraprado]

I think it would make sense to have a special store command, as you suggest. with behaviour:

* If it is given no argument, it expects to find the name of the variable on the top of the stack (as you suggest).

* But it can be given an optional variable name, in which case the value(s) are stored into that variable.

I'm not sure I understand how the command could be given an optional variable name without passing it through the stack. Would it be passed through the modifiers field, like store:'my_var'? Is that possible?

store could follow the convention of acting on just the top stack element unless you said :3 or :* in which case a list would be stored.

I'm not sure if the value(s) should then be popped off the stack, but I think they should (which matches dc). The user could use the regular = modifier to leave the stack as it was (in which case the value would still be in the variable).

Sounds great to me!

[terrycojones]

Yeah..... I'm just looking at the code and I don't see how to do that (cleanly) either. So I suggest we just start with the simple case (variable name on the stack) and see how that works. You can do it if you like, though I am also doing it :-)

[terrycojones]

OK, have a look at #7

BTW, I think the default way of putting args onto the stack feels really backwards! It feels to me much more natural to think only afterwards that you want to store something into a variable, or that you want to join some strings or apply some function. You can use the :r modifier to do it that way (works with store too) but I now think that should have been the default and that I was being too strict in saying you should push things on in the exact order that they would be given in a regular function call. E.g., because you write map(str.lower, ('ONE', 'TWO')) you should therefore push str.lower first, then ONE, etc. While that might make sense for map (it arguably does) it feels wrong for store and join.

Anyway, see what you think of this. If you don't want to fetch it & test it out I'll just merge it & you can update and try it that way.

Thanks!

[terrycojones]

Actually, I think I see where I did it wrong. When working with RPN you do things from the inside out, so for 6 * ( 5 + 4) you'd push the 5, push the 4, do the +, push the 6, do the *. From that POV, with map(str.lower, ('ONE', 'TWO')) you should push the ONE then the TWO (either separately or as a single list), then push str.lower, then push map then call apply. Same goes for join: push the things to be joined, then call 'separator'.join on them (so the string to join on is specified last). And same for store.... you do something and then you decide to store it (and dream up a variable name at that point, not in advance).

So I think this thing has its default wrong.

[nfraprado]

Indeed what I had in mind was to pass the variable name last, since as you said it, you normally realize that you want to save the value to a variable at that time (Also, if the variable was passed first, to save the whole stack you would need to start by pushing the variable name, which doesn't make much sense to me). So the function should really be def setVariable(self, value, variable).

And no need to merge it right now, I can test from your branch no problem.

[terrycojones]

OK, but the setVariable function isn't callable from the calculator, so that's just regular Python, and nothing to do with RPN.

I think we should put out a version 2 that changes the default argument order.

[nfraprado]

Oh, I see. I hadn't looked closer at the code yet. What makes this "variable name before variable value" order is _findWithArgs which has that baked into.

So what you're suggesting is to change the order assumed by _findWithArgs, so that the item is the one on top rather than on the bottom of the stack as part of the PR? Won't that break things though?

[nfraprado]

Actually essentially what needs to be done is to switch the if modifiers.reverse with its else, right? So we're making rpnpy behave like :r is the default, and then passing :r would make it behave like it does now, so if that breaks anything would probably be just the tests which have to be reversed.

All in all this seems very good as it makes the default behavior more postfix-like.

[terrycojones]

Actually essentially what needs to be done is to switch the if modifiers.reverse with its else, right? So we're making rpnpy behave like :r is the default, and then passing :r would make it behave like it does now, so if that breaks anything would probably be just the tests which have to be reversed.

All in all this seems very good as it makes the default behavior more postfix-like.

Yes, that's what I'm suggesting. I don't know if the code change would be that simple. Many tests would break (which is fine). I'd bump up the major version number since this is backwards incompatible.

I would also consider just getting rid of the :r modifier (though I probably would leave it in) because I can tell now that the reason I put it in was because I had things the wrong way around to begin with :-)

[terrycojones]

OK, I think I'll merge this branch, assuming you think it's ok.

And I'm going to open a ticket to reverse the default arg order. I could add a command-line option to let people keep the old behaviour. Though I don't know if there any other users in the whole world! :-) It would make things quite messy (IMO). Let's see.

[nfraprado]

Yeah, go ahead and merge it.

The reversed default arg order makes me more confused the more I think of it hahaha. But I guess that's a separate issue and I can think it over and we can discuss it better in its own issue.

The store command added in this PR shouldn't have any impact on that, and looks great to me. Also, thanks for going ahead and implementing it :).

[terrycojones]

OK, merged & uploaded. Thanks! I've realized that what I did last night re reversing makes things feel more comfortable but is actually inconsistent. Anyway, will say more over in #8 at some point.