Homoiconic REPL #1246

Kodiologist · 2017-03-06T00:35:07Z

This pull request adds a function hy-repr and uses it to print out stuff in the REPL (edit: only when you launch Hy with --hy-repr).

Closes #351.

#1244 is a prerequesite. Merge that first and I'll rebase this.

An annoying consequence of this change (edit: now fixed, using the method listed second below) is that setv at the REPL now prints the value of whatever you assigned. I think this is bad because when I'm doing data analysis on the REPL, I often say things like (setv x (some-function-that-produces-a-huge-pandas-dataframe)), and I don't want that printed. I see two ways around this:

Make setv always return None. The question of what setv should return, particularly in the REPL, is kind of a mess, anyway (setv on tuple returns the tuple? #249, inconsistent return behavior for setv in repl #871, Do we *really* want setv to return a tuple? #1049), so this change would eliminate another weird corner of the language.
Make the REPL check whether the last statement of each input is ast.Assign, and if so, don't print the value.

As for the documentation examples, I don't think we should bother systematically updating them before we have documentation tests.

refi64 · 2017-03-06T16:32:24Z

Ehhh...I'm not really a fan of this one...

IMO part of what I love about Hy is that it's just Python, except different. If someone starts using Hy, they likely already know Python. Using the Python syntax in the repr isn't going to be confusing.

More importantly, though, this only half-way works with Python classes. Imagine a Python class that reprs with Python collection syntax. You'd probably end up having to look at something like this:

#{MyClass(val={1, 2, 3})

which is worse than the current behavior.

In addition:

The prepending of (do is going to make column numbers off. They have to be fixed.
This changes the behavior of --spy: if an exception is thrown, then the AST is never printed. IMO this removes a lot of its usefulness for me...

Both of these could be fixed by separating the parsing and code execution (which would allow you to put (do at the AST level).

Yeah...not a big fan... :O

refi64

See comment I left (too lazy to copy-paste).

Kodiologist · 2017-03-06T16:45:36Z

Are you saying you would approve this if I made the changes you've requested, or not?

I don't understand your point or your example about "this only half-way works with Python classes". Can you be a bit more explicit?

The prepending of (do is going to make column numbers off.

Can you give an example of how this appears to the user?

gilch · 2017-03-06T19:33:48Z

I'm also 👎 on this. This has come up before #220 #351 #354. Python's reprs in general are never going to work in Hy. We can adjust our own libraries, but not the whole Python ecosystem. We could try to auto-convert reprs from Python, but I don't think it'll ever be 100%. Often the reprs of collection-like objects defer to the reprs of what they contain. Remember that Hy libraries can also be used from Python. I'd rather have valid Python reprs in all cases than some weird hybrid of the two that doesn't work consistently in either Hy or Python. That means we should adjust Hy's reprs toward Python, not the other way around.

gilch · 2017-03-06T19:40:18Z

I just thought of a weird compromise, and I'm not sure how I feel about it. If we implemented some kind of Hy string eval that we could invoke from Python, then we could make valid Python reprs that still contain Hy code:

=> '(1 2 3)
hy_eval("'(1 2 3)")

gilch · 2017-03-06T19:46:38Z

Ah, no. I don't think that's good enough. You'd also need the reverse, Python, eval, since the collection can only represent objects by their Python repr, so:

=> '(1 2 {3 4})
hy_eval("'(1 2 (eval \"{3: 4}\")")

Nested quotes like that could get gnarly quickly. I don't think that idea is workable either.

Kodiologist · 2017-03-06T19:48:13Z

Python's reprs in general are never going to work in Hy.

Yes, that's one reason I think that this change is better than trying to make more Python syntax into valid Hy syntax.

We could try to auto-convert reprs from Python, but I don't think it'll ever be 100%.

Yeah, that sounds like a bad idea.

Remember that Hy libraries can also be used from Python.

Yes, that's why __hy-repr__ is different from __repr__.

I'd rather have valid Python reprs in all cases

We don't already have that, nor is it likely to be possible. For example, at the Python prompt, x = [1]; y = [2, 3]; y[0] = x; y[1] = x; y prints [[1], [1]] even though the two elements of y are the same object. And lambdas get repred as <function <lambda> at 0xdeadbeef> instead of disassembled. hy-repr, like repr, can only be an approximation.

That means we should adjust Hy toward Python, not the other way around.

That would seem to require making Python syntax into valid Hy syntax, which we just rejected.

I don't like your compromise because it will add a lot of visual noise to REPL output, which needs to be easily readable to serve its function of convenience.

refi64 · 2017-03-06T20:08:06Z

Are you saying you would approve this if I made the changes you've requested, or not?

Depends on what everyone else says.

I don't understand your point or your example about "this only half-way works with Python classes". Can you be a bit more explicit?

`foo.py`

class Foo:
    def __init__(self):
        self.items = [1, 2, 3]
    def __repr__(self):
        return 'Foo(items=%r)' % self.items

in repl

hy 0.12.1+28.g484daaf.dirty using CPython(default) 2.7.6 on Linux
=> (import foo)
=> #{(foo.Foo)}
#{Foo(items=set([1, 2, 3]))}
=>

It's inconsistent. And it'll stay that way unless random library authors add __hy_repl__ implementations (but they probably won't).

Can you give an example of how this appears to the user?

hy 0.12.1+28.g484daaf.dirty using CPython(default) 2.7.6 on Linux
=> ]  File "<stdin>", line 1, column 5

  (do ])
      ^
LexException: Ran into a RBRACKET where it wasn't expected.



=>

Kodiologist · 2017-03-06T20:14:13Z

It's inconsistent. And it'll stay that way unless random library authors add __hy_repl__ implementations (but they probably won't).

It seems that our options are either to allow the inconsistency or to use a generic placeholder syntax for classes with no __hy_repr__, which merely names the class. To me the inconsistency seems a better option, since it provides more information.

In your example, the column number and the caret position seem to be correct.

gilch · 2017-03-06T20:15:40Z

That means we should adjust Hy toward Python, not the other way around.

That would seem to require making Python syntax into valid Hy syntax, which we just rejected.

You're not getting my meaning. I meant that things like '(1 2) should print out something like HyExpression((([] + [HyInteger(1)]) + [HyInteger(2)])), or <Hy '(1 2)> instead of just '(1 2), which makes no sense from Python.

Kodiologist · 2017-03-06T20:18:09Z

HyExpression((([] + [HyInteger(1)]) + [HyInteger(2)])) seems like more of a problem than a solution, but <Hy '(1 2)> seems pretty reasonable.

gilch · 2017-03-06T20:28:54Z

Yeah, I only wrote it that way because that's what came out of the AST. That really could be simplified in the repr to HyExpression([HyInteger(1), HyInteger(2)]), or even hy([hy(1), hy(2)]) if we had a hy function that dispatches to the right model based on the data type it receives. The problem with something like <Hy '(1 2)> is how to nest them properly. With a dispatch function, it's easy, and it might even work in the Python repl.

Kodiologist · 2017-03-06T20:43:41Z

In any case, I think that what you're proposing is orthogonal to this PR. You're proposing how to make Python representations for Hy models, and this PR is about Hy representations for all sorts of values, including Hy models. The only way to see any of this PR's changes in the Python REPL would be if you did from hy.core.language import hy_repr; hy_repr(whatever).

Kodiologist · 2017-03-07T21:25:08Z

@hylang/core Anybody else want to chime in on this? (Assume I'll be able to fix the bug Ryan mentioned that --spy doesn't work when the code raises an exception. I don't anticipate any problems there.) I think it's obviously a big improvement on what we had. But if nobody else likes it, I can submit another PR with just the first of these two commits, adding hy-repr but not using it automatically in the REPL.

Kodiologist · 2017-03-07T21:29:37Z

Another idea is to have a command-line option that enables use of hy-repr in the REPL. However, making this feature the default seems wise to me because it will make the REPL much less surprising for newbies.

Kodiologist · 2017-03-13T00:28:45Z

@gilch @kirbyfan64 What do you think of using a command-line option for this, as I mentioned above? If you're against the feature being on by default (with an option to disable it), how about off by default (with an option to enable it)?

@tuturto Can you chip in?

refi64 · 2017-03-13T01:46:46Z

I guess I wouldn't mind it if it were disabled by default...

That being said, what if we put a prefix? e.g prefixing the command with something (no clue what yet) would use this repr...or maybe a reader macro?

Kodiologist · 2017-03-13T01:53:50Z

Then you'd have to remember to use the prefix or reader macro or whatever for every command, which again reduces newbie accessibility.

gilch · 2017-03-13T03:55:57Z

What do you think of using a command-line option for this

That being said, what if we put a prefix? e.g prefixing the command with something (no clue what yet) would use this repr...or maybe a reader macro?

This is the kind of thing other Lisps would use a dynamic variable for hylang/hyrule#51. It would be nice to toggle other REPL features (like --spy) this way.

Still, I do not approve.

Even with a separate hy-repr and repr, you can still get weird hybrid representations that are neither fully Python nor fully Hy. This is because the __repr__ implementation of collection and wrapper types often defer to the repr of what they contain. The __hy-repr__ implementations of collection and wrapper types written in Hy would have to do the same, since hy-repr has to default to just repr when there is no __hy-repr__ implementation--which there won't be for the whole Python ecosystem, save for our own little corner.

It's also more work to implement both a __repr__ and a __hy-repr__ for every class.

Thus spake @paultag

Hy's failures mostly stem from us trying to cover up the fact it's actually Python

We should expect Hy users, even newbies, to know Python, at at least a basic level--just as Clojure(Script) users must understand Java(Script). I do not approve of trying to hide the Python that is in Hy, since this is the root of many of Hy's difficulties. I might even be helpful for newbies if we turned on --spy in the repl by default.

Kodiologist · 2017-03-13T04:12:52Z

Even with a separate hy-repr and repr, you can still get weird hybrid representations that are neither fully Python nor fully Hy.

Like I said, there's no reason one couldn't put a generic placeholder in place of an object with no __hy-repr__; it just seems that the Python representation is more likely to be useful. Some __repr__s themselves aren't Python code, like Pandas data frames.

I do not approve of trying to hide the Python that is in Hy

Spitting out values in a syntax that you can actually use as input is hiding? I mean, you don't think the fact that the input syntax isn't Python is hiding, do you?

tuturto · 2017-03-13T04:45:36Z

Long and good discussion. I love seeing people hashing out some hard problem respectfully ❤️

I'm having hard time deciding either way though. I would like the idea that I can just copy/paste repl output back to input and have it work. But I'm bit hesitant if it would be confusing to people (old and new to Hy alike) that it sometimes works and sometimes doesn't.

Lets run an experiment: we could bring this in, have it disabled by default, activated by command line switch and see how it behaves in the wild. After some amount of time, we can get back to the matter and see what we learned. At that point we can adjust things or even remove it completely (as if any project would remove a feature, ever) if this turned out to be a bad idea. How does this sound like?

I didn't look into code really. Just got up and had interesting looking notifications from GitHub waiting 😁

Oh, and can we fix REPL printing out what setv was used to set? And have --spy print out ast even when exception is thrown?

Kodiologist · 2017-03-13T04:48:17Z

I don't think that's a bad idea. We can at least mention it in the tutorial to maximize the chance it will actually see use in the wild.

Kodiologist · 2017-03-13T20:06:48Z

Okay, I fixed the --spy bug, added a command-line option, added tests, etc. Take a look. Don't squash, since the middle commit is mostly unrelated.

tuturto · 2017-03-19T20:21:08Z

hm.. true. No need for the shortcut.

gilch · 2017-03-20T17:31:58Z

Would you approve hy --repl-output-fn hy.contrib.hy-repr.hy-repr, then?

I would not be at all opposed to modularizing the repl. Once that's done, I wouldn't be particularly opposed to contrib experiments taking advantage of it. That invocation does seem long-winded (though I suppose one can always use aliases or shell scripts for such things).

I wonder what a good interface would be. Perhaps --read, --eval, and --print options would be OK. Or maybe we could just point it to a Hy (or Python) module that exports some subset of Read, Eval, and/or Print callables that can override the defaults. Then it would just be

$ hy --repl hy.contrib.hy-repr

If you want to mix-and match, say Read from one module and Print from another, then you have to write a module that imports each from wherever. Other future repl implementations (GUI? emacs?) could also use the same config module.

gilch · 2017-03-20T17:41:44Z

I can think of another advantage of the config module approach. You could set state in the module through a special repl command in the Read callable that swaps out the implementation used by one of the other callables without the need to restart the repl. You could even set it up to import new ones on the fly.

Kodiologist · 2017-03-20T17:44:08Z

Okay. In the spirit of YAGNI, I'll just implement --repl-output-fn and move the hy-repr function to hy.contrib for now.

Kodiologist · 2017-03-20T19:43:37Z

@gilch Done.

tuturto · 2017-03-24T04:30:00Z

Also, realized that macroexpand is really nice now:

=> (macroexpand '(if (= a 1) "one" (= a 2) "two" "many"))
'(if* (= a 1) "one" (if (= a 2) "two" "many"))

Somehow missed this fact while testing earlier.

….hy-repr)

Closes hylang#1255.

Kodiologist · 2017-03-24T16:35:48Z

Okay, this should finally be ready to go.

refi64 · 2017-03-24T17:45:19Z

hy/cmdline.py

+                tokens = tokenize(source)
+            except PrematureEndOfInput:
+                return True
+            tokens = tokenize("(do " + source + "\n)")


Last thing, I swear!! :O

Would this be better as:

tokens = tokenize(source) do = HyExpression([HySymbol('do')] + tokens) do.start_line = do.end_line = do.start_column = do.end_column = 1 do.replace(do)

that way the (do ...) doesn't appear in the stack traces/compile errors?

In the current implementation, (do ...) already doesn't appear. (Your example above no longer has do in its output.)

Hmm, but the underline from a HyMacroExpansionError seems to be misaligned. I'll take a look at it.

refi64 · 2017-03-24T19:29:04Z

@tuturto Do you have any objections since you last approved 8 days ago, or are we good to go?

tuturto · 2017-03-25T11:49:52Z

@kirbyfan64 we're good to go

This reverts commit 5db0fbf.

Kodiologist · 2017-03-25T15:25:56Z

Thanks, folks. I redid the merge without squashing so the changes between commits are differentiated.

Kodiologist force-pushed the hy-repr branch 3 times, most recently from 2f4270f to 488494f Compare March 6, 2017 01:07

refi64 requested changes Mar 6, 2017

View reviewed changes

Kodiologist force-pushed the hy-repr branch from 488494f to 165576b Compare March 6, 2017 16:58

Kodiologist force-pushed the hy-repr branch 3 times, most recently from 14d6ff6 to fbf87b5 Compare March 13, 2017 20:05

Kodiologist force-pushed the hy-repr branch from 16d1bb5 to 1f2d2d8 Compare March 20, 2017 19:36

Kodiologist mentioned this pull request Mar 21, 2017

A strange operation for "as->" macro. #1255

Closed

Kodiologist force-pushed the hy-repr branch 2 times, most recently from 5e5070a to 2611723 Compare March 21, 2017 23:23

Kodiologist mentioned this pull request Mar 23, 2017

Make setv always return None #1257

Merged

Kodiologist added 4 commits March 24, 2017 08:43

Refactor test_bin

32e76ca

Add hy.contrib.hy-repr

bf2f90a

Add a command-line option --repl-output-fn (especially for hy.contrib…

33a696d

….hy-repr)

Add a test for as-> in the REPL

ca1bd0f

Closes hylang#1255.

Kodiologist force-pushed the hy-repr branch from 2611723 to ca1bd0f Compare March 24, 2017 16:05

refi64 reviewed Mar 24, 2017

View reviewed changes

Fix HyMacroExpansionError underline alignment

e478008

refi64 approved these changes Mar 24, 2017

View reviewed changes

refi64 merged commit 5db0fbf into hylang:master Mar 25, 2017

Kodiologist added a commit that referenced this pull request Mar 25, 2017

Revert "Homoiconic REPL (#1246)"

912337b

This reverts commit 5db0fbf.

Kodiologist deleted the hy-repr branch March 26, 2017 23:33

refi64 mentioned this pull request Apr 16, 2017

Assoc returning the dict #238

Closed

Kodiologist mentioned this pull request Aug 7, 2017

Empty (?) reference to a function definition in a list of expressions. #1372

Closed

Kodiologist mentioned this pull request Mar 28, 2018

Move hy-repr to core and rename eval to hy-eval #1550

Closed

Kodiologist mentioned this pull request Apr 7, 2018

Burninate HyCons #1576

Closed

Homoiconic REPL #1246

Homoiconic REPL #1246

Conversation

Kodiologist commented Mar 6, 2017 • edited Loading

refi64 commented Mar 6, 2017

refi64 left a comment

Choose a reason for hiding this comment

Kodiologist commented Mar 6, 2017

gilch commented Mar 6, 2017 • edited Loading

gilch commented Mar 6, 2017 • edited Loading

gilch commented Mar 6, 2017 • edited Loading

Kodiologist commented Mar 6, 2017

refi64 commented Mar 6, 2017

foo.py

in repl

Kodiologist commented Mar 6, 2017

gilch commented Mar 6, 2017 • edited Loading

Kodiologist commented Mar 6, 2017

gilch commented Mar 6, 2017 • edited Loading

Kodiologist commented Mar 6, 2017 • edited Loading

Kodiologist commented Mar 7, 2017

Kodiologist commented Mar 7, 2017

Kodiologist commented Mar 13, 2017

refi64 commented Mar 13, 2017

Kodiologist commented Mar 13, 2017

gilch commented Mar 13, 2017 • edited Loading

Kodiologist commented Mar 13, 2017

tuturto commented Mar 13, 2017 • edited Loading

Kodiologist commented Mar 13, 2017

Kodiologist commented Mar 13, 2017

tuturto commented Mar 19, 2017

gilch commented Mar 20, 2017 • edited Loading

gilch commented Mar 20, 2017

Kodiologist commented Mar 20, 2017 • edited Loading

Kodiologist commented Mar 20, 2017

tuturto commented Mar 24, 2017

Kodiologist commented Mar 24, 2017

refi64 Mar 24, 2017

Choose a reason for hiding this comment

Kodiologist Mar 24, 2017

Choose a reason for hiding this comment

Kodiologist Mar 24, 2017

Choose a reason for hiding this comment

Kodiologist Mar 24, 2017

Choose a reason for hiding this comment

refi64 commented Mar 24, 2017

tuturto commented Mar 25, 2017

Kodiologist commented Mar 25, 2017

Kodiologist commented Mar 6, 2017 •

edited

Loading

gilch commented Mar 6, 2017 •

edited

Loading

gilch commented Mar 6, 2017 •

edited

Loading

gilch commented Mar 6, 2017 •

edited

Loading

`foo.py`

gilch commented Mar 6, 2017 •

edited

Loading

gilch commented Mar 6, 2017 •

edited

Loading

Kodiologist commented Mar 6, 2017 •

edited

Loading

gilch commented Mar 13, 2017 •

edited

Loading

tuturto commented Mar 13, 2017 •

edited

Loading

gilch commented Mar 20, 2017 •

edited

Loading

Kodiologist commented Mar 20, 2017 •

edited

Loading