hauntsaninja / pyp

Easily run Python at the shell! Magical, but never mysterious.
MIT License
1.41k stars 39 forks source link

Print last defined variable if no print statements and no final value #12

Closed dbieber closed 4 years ago

dbieber commented 4 years ago

A few feature requests:

  1. If the pyp command ends with an assignment, use the value of that assignment as the value to print.
  2. textwrap.dedent before parsing
  3. Allow import aliases

Note that 3. is different from the discussion we've been having in #5 and #6. If the user explicitly imports something (something they may not want to include in their config file once that exists), that should work.

E.g. I think this should work:

echo 10 | pyp '
  import numpy as np  # Ideally this line isn't needed either
  values = range(1, int(x))
  unnormalized_probabilities = [1/value for value in values]
  total = sum(unnormalized_probabilities)
  probabilities = [p / total for p in unnormalized_probabilities]
  value = np.random.choice(values, p=probabilities)
'

Whereas today this is needed:

echo 10 | pyp '
values = range(1, int(x))
unnormalized_probabilities = [1/value for value in values]
total = sum(unnormalized_probabilities)
probabilities = [p / total for p in unnormalized_probabilities]
value = numpy.random.choice(values, p=probabilities)
value
'

Note the three changes that were needed to address each of the three feature requests.

  1. added value line at end
  2. manually dedented
  3. used numpy in place of import alias

The reason for preferring the former is it makes it relatively easy to reuse code from a real Python file with minimal modifications for pyp.

Wdyt about these requests?

hauntsaninja commented 4 years ago

Thanks again for helping make pyp better!

Issue 3 is a bug, fixed by https://github.com/hauntsaninja/pyp/commit/d19e30b48244214918e53474c3ea377e4d145136

Issue 2 seems a reasonable change to make. Is this to make copy-pasting easier?

I'm a little worried about issue 1, since an unassigned expression seems to convey better the intent of "someone do something with this!". Why assign in the first place? If you're using Python 3.8 or later, you can also use a named expression.

dbieber commented 4 years ago

Yes all three of these are to make copy-pasting from an existing or WIP codebase easier.

Issue 3 is a bug, fixed by d19e30b

Great!

Issue 2 seems a reasonable change to make.

👍

Issue 1. Why assign in the first place?

Your concern is understandable. The reason for assigning it in the first place is b/c it's a snippet copied from a larger Python file. The purpose of the request is to minimize the impedance between developing a larger codebase in an editor and quickly testing or using snippets with pyp.

dbieber commented 4 years ago

I also have a fourth feature request that would minimize this impedance still further, but I think you probably will choose not to take it because it makes backwards compatibility difficulty.

  1. If there's a single unbound variable, assume it is meant to be the input.

This has the potential to require breaking backwards compatibility in the future though if you want to add new keywords.

hauntsaninja commented 4 years ago

If I'm understanding correctly, that would break things like pyp 'requests.get(...)', since we import undefined names. It might also veer on the side of too magical: right now if users have a genuine NameError in their code, it will mostly just get mapped to an ImportError and fail upfront. That makes it pretty easy to understand, but if you start getting weird TypeErrors it's trickier. Constraining the magic to a couple names makes mental models more predictable.

hauntsaninja commented 4 years ago

Sorry, got closed as part of Github's workflow. Where this stands: issue 2 and 3 are now fixed on master, issue 4 is a no (as I understand it). I'm still open to issue 1, especially if there are other requests — let me sit on it a little :-)

naclander commented 4 years ago

I wanted to throw in my opinion that implementing issue 1 does not seem like the right thing to do here. Especially as with the first example, simply not using value = would get the job done here.

hauntsaninja commented 4 years ago

Thanks for weighing in!

@dbieber I'm probably not going to add this feature. However, if this is something that is critical for your workflow, the following patch of pyp should accomplish it:

diff --git a/pyp.py b/pyp.py
index 0bd5f01..b4f8ebb 100644
--- a/pyp.py
+++ b/pyp.py
@@ -285,7 +285,7 @@ class PypTransform:
             if isinstance(body[-1], ast.Pass):
                 del body[-1]
                 return True
-            if not isinstance(body[-1], ast.Expr):
+            if not isinstance(body[-1], (ast.Expr, ast.Assign)):
                 # If the last thing in the tree is a statement that has a body (and doesn't have an
                 # orelse, since users could expect the print in that branch), recursively look
                 # for a standalone expression.
@@ -293,7 +293,11 @@ class PypTransform:
                     return inner(body[-1].body, use_pypprint)  # type: ignore
                 return False

-            if isinstance(body[-1].value, ast.Name):
+            if isinstance(body[-1], ast.Assign):
+                if len(body[-1].targets) > 1 or not isinstance(body[-1].targets[0], ast.Name):
+                    return False
+                output = body[-1].targets[0].id
+            elif isinstance(body[-1].value, ast.Name):
                 output = body[-1].value.id
                 body.pop()
             else:
dbieber commented 4 years ago

Thanks for the patch and for considering my ideas. Certainly Issue 1 is not critical -- just a small time saver. And glad to see Issues 2 + 3 resolved :).