Skip to content

Conversation

ihanson
Copy link
Contributor

@ihanson ihanson commented Aug 2, 2012

See http://ask.sagemath.org/question/1647/sagecell-kwargs-no-longer-works for issues that this creates.

The following should have the same behavior, but it doesn't.

@interact
def f(n = ContinuousSlider()):
    print locals()
    print globals()["n"]


@interact(controls = [("n",ContinuousSlider())])
def f(**kwargs):
    print locals()
    print globals()["n"]

What seems to be the issue is that when defining interact controls in the interact decorator, we add those to the interacted function's global namespace, rather than in the local namespace (see the difference between adapted_f() in the previous version of interact_sagecell.py in comparison to the current version in /contrib/ipython_testing.).

I seem to remember that we talked about this potential issue briefly a while back, but I don't think it was ever resolved.

(no longer using function globals to pass these values)
Also allows a user to specify a named argument to an interact
function through which the value will be passed
@ihanson
Copy link
Contributor

ihanson commented Aug 2, 2012

This pull request fixes the issue, by reverting back to the old behavior of passing values through kwargs.

It also allows the user to do something like this as an alternative:

@interact(controls = [("n",ContinuousSlider())])
def f(n):
    print locals()
    print globals()["n"]

@jasongrout
Copy link
Member

Thanks!

@jasongrout
Copy link
Member

Some comments:

  1. The controls setting comes before the args in the function, but that isn't natural-looking:
@interact(controls = [("n",ContinuousSlider())])
def f(s='hi',n=None):
    print s,n

It looks like s should come first in the controls, but n comes first.

  1. Also, I have to specify a default value for n above since I put s first, but then it's confusing because the default value in f is not controlling the control widget. In fact, (I think) any default I give for n in the definition of f is ignored.

I think for now, it would be better to change things back to the way they were even more:

  1. function args should added to the list of args before the controls keyword
  2. What was done before if there was a conflict between controls names and function arguments? If you know the names at the time of function definition, then just put them in the function definition. If you don't, then you can't put them in the function definition anyway. So it seems that it would be a simpler interface if an error was thrown if a controls attribute had the same name as a function argument.

Raise an error if there are duplicate arguments defined for the interact
@jasongrout
Copy link
Member

This looks much better; thanks! I put in one minor comment above.

@jasongrout
Copy link
Member

Never mind; I was being dumb. When the control is specified twice in the controls list, you're right that we want to throw an error too.

jasongrout added a commit that referenced this pull request Aug 3, 2012
Defining variables in interact decorator defines in globals, not locals.  This also makes @interact(controls=...) pass the controls into the function's **kwargs argument.
@jasongrout jasongrout merged commit 8c24fb6 into sagemath:master Aug 3, 2012
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.

2 participants