allow for csp nodes and graphs to be defined in repl

[timkpaine]

Fixes #177 by reassembling the source code from the interpreter history

Needs tests

[timkpaine]

[timkpaine]

could be a little smarter here and go block-by-block to ensure we only grab 1

[timkpaine]

I tried 3 different ways to add tests:

• exec -> uses <string>, added a code path to raise a more meaningful error
• python -c -> also uses <string>
• printf | python -> could not exit cleanly and tough to wrap in a test

None worked nicely.

[robambalu]

would using dill.source.getsource() simplify the changes needed on our end?
I'm hoping it can just be as simple as calling dill.source.getsource() if inspect.getsource fails

[timkpaine]

would using dill.source.getsource() simplify the changes needed on our end? I'm hoping it can just be as simple as calling dill.source.getsource() if inspect.getsource fails

i had hoped so too but there appears to be a bug in dill getsource (which i tried to repro and fix). I can try again to repro/fix it but:

• dill is an external dependency and the getsource code does a lot more than this (though it also handles a lot more cases than this)
• if we made the upstream fix we'd be bound to a relatively new version of dill

The bug:

  File "<unknown>", line 2
    def foo(x: csp.ts[int]) -> csp.ts[int]:
                                           ^
IndentationError: expected an indented block

uqfoundation/dill#603

I believe it is a fencepost error and omitting the last line.

[timkpaine]

uqfoundation/dill#651

[timkpaine]

My patch to dill has been merged upstream, we can either require users that want this functionality to have a very latest dill, or keep the code in our own repo as the PR does currently.

Since defining nodes and graphs in a repl is not super common or practical, I think either option is fine.

[ptomecek]

My patch to dill has been merged upstream, we can either require users that want this functionality to have a very latest dill, or keep the code in our own repo as the PR does currently.

Since defining nodes and graphs in a repl is not super common or practical, I think either option is fine.

I'm a proponent of getting this working one way or another, as I think it improves the user experience (and maybe also because I try to define nodes in the repl more than most users). I don't have a super strong view about how it's done (i.e. force a relatively new version of dill in the dependencies or include the code in the repo), but I am in favor of having it work "by default", i.e. without users needing to opt-in (i.e. by manually installing dill).

@robambalu Which way are you leaning?

[robambalu]

My patch to dill has been merged upstream, we can either require users that want this functionality to have a very latest dill, or keep the code in our own repo as the PR does currently.
Since defining nodes and graphs in a repl is not super common or practical, I think either option is fine.

I'm a proponent of getting this working one way or another, as I think it improves the user experience (and maybe also because I try to define nodes in the repl more than most users). I don't have a super strong view about how it's done (i.e. force a relatively new version of dill in the dependencies or include the code in the repo), but I am in favor of having it work "by default", i.e. without users needing to opt-in (i.e. by manually installing dill).

@robambalu Which way are you leaning?

I would go with dill so we dont have to maintain this extra code, as long as we can tell when we're in a case that dill is required and we can then do a version check / raise an exception if its not supported

[ptomecek]

My patch to dill has been merged upstream, we can either require users that want this functionality to have a very latest dill, or keep the code in our own repo as the PR does currently.
Since defining nodes and graphs in a repl is not super common or practical, I think either option is fine.

I'm a proponent of getting this working one way or another, as I think it improves the user experience (and maybe also because I try to define nodes in the repl more than most users). I don't have a super strong view about how it's done (i.e. force a relatively new version of dill in the dependencies or include the code in the repo), but I am in favor of having it work "by default", i.e. without users needing to opt-in (i.e. by manually installing dill).
@robambalu Which way are you leaning?

I would go with dill so we dont have to maintain this extra code, as long as we can tell when we're in a case that dill is required and we can then do a version check / raise an exception if its not supported

That's fine with me, as long as it's super clear to users what they need to do when they encounter the error. Maybe down the road we can include the new-enough dill as part of the requirements, but I agree it's probably too new to do that now.