Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove car/cdr in favor of first/rest #909

Closed
gilch opened this issue Aug 18, 2015 · 7 comments · Fixed by #1241
Closed

Remove car/cdr in favor of first/rest #909

gilch opened this issue Aug 18, 2015 · 7 comments · Fixed by #1241

Comments

@gilch
Copy link
Member

gilch commented Aug 18, 2015

One of the subissues from #240. I think we'll make more progress discussing one subissue at a time.

rest / cdr are not quite the same, but still redundant. (cdr coll) is a macro that expands into a slice like coll[1:]. rest is a partial application of the function islice that does about the same thing as cdr. As a function, rest can be assigned to variables, passed to other functions, etc. As a macro, cdr cannot. Slices also don't work on generators so cdr doesn't work, but islices do, and so does rest. rest will do in almost all cases we currently use cdr.

The main problem with replacing all slices with islices, is that islice will always be a generator, but slices return another instance of the same collection type. Most of the time this isn't a problem. Python's libraries are very good about generally accepting iterables where you might have used a collection before. The main exception is strings, which really need to stay as strings.

If you do run into a case like this, you can still use (cut coll 1) for the same effect.

The case for dropping car in favor of first is even stronger. car is a macro like coll[0], while first is a function like next(islice(coll, 0, None)), which should perhaps be simplified to next(iter(coll)). Either way, you get the first element. Except car doesn't work on generators, and can't be passed as a higher-order function, etc. There is absolutely no reason to keep car except for symmetry with cdr, which we should also drop.

@farhaven
Copy link
Contributor

Can you open a PR for this? 👍 from me.

@gilch
Copy link
Member Author

gilch commented Aug 19, 2015

I'm getting a strange "internal compiler bug" error I haven't been able to pin down when I try to replace list-comp with genexpr in #903.

hy.errors.HyCompileError: Internal Compiler Bug \U0001f631
\u2937 TypeError: Don't know how to wrap a <class 'generator'> object to a HyObject

(long stack trace omitted for brevity)

I'm worried that attempting to remove cdr will cause similar blowups if this isn't fixed first.

This is the version that causes the error:

(defmacro defmacro! [name args &rest body]
  "like defmacro/g! plus args that start with o! will automatically evaluate
   once only and be bound to the equivalent g!"
  (setv os (genexpr s [s args] (.startswith s "o!"))
        gs (genexpr (HySymbol (+ "g!" (cut s 2))) [s os]))
  `(defmacro/g! ~name ~args
     `(do (setv ~@(interleave ~gs ~os))
          ~@~body)))

I'm not absolutely certain that this is an error in Hy, and not just in defmacro!, but the version with list-comp seems to work:

(defmacro defmacro! [name args &rest body]
  "like defmacro/g! plus args that start with o! will automatically evaluate
   once only and be bound to the equivalent g!"
  (setv os (list-comp s [s args] (.startswith s "o!"))
        gs (list-comp (HySymbol (+ "g!" (cut s 2))) [s os]))
  `(defmacro/g! ~name ~args
     `(do (setv ~@(interleave ~gs ~os))
          ~@~body)))

@refi64
Copy link
Contributor

refi64 commented Aug 19, 2015

Well, the error could be a bit nicer (!), but the reason is simple. Hy automatically converted the list returned from list-comp into a HyList model. It doesn't know how to do that with generators and therefore proceeds to explode. :)

@gilch
Copy link
Member Author

gilch commented Aug 20, 2015

A lot of Python's sequence manipulation tools return generators, not lists. You need sequence manipulation (LISt Processing) in macros most of all, but you're telling me that they're completely incompatible? It would be a pain to have to use (list ...) calls all the time in macro definitions. I'm not even sure I understand where they're required, and the error message isn't helping there.

Can we teach Hy to "do that" with generators? Clojure seems to be able to use lazy sequences without this problem. Maybe a HyModel for anything not already covered with hasattr(object,"__iter__")?

@refi64
Copy link
Contributor

refi64 commented Aug 20, 2015

I prefer being explicit here. It's happened to me before where I passed the wrong type that happened to be an iterable to an API that implicitly converted it to a list.

@gilch
Copy link
Member Author

gilch commented Aug 20, 2015

I'm a lot more confused with the way Hy isn't working now than I would be if something implicitly became a list, and like I said, I haven't run into this kind of problem with Clojure's lazy sequences. At the very least this needs to be better documented. I still don't understand what's triggering this error. Can you demonstrate with a minimal example?

@Foxboron
Copy link
Member

Essentially, this is not being used; https://github.com/hylang/hy/blob/master/hy/models/__init__.py#L39
We wrap the different types into HyModels, but they don't always work very well. Thats why you sometimes see people explicitly using HyModels inside macros.

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 a pull request may close this issue.

4 participants