Give `require` the same features as `import` #1142

Kodiologist · 2016-10-27T21:10:22Z

You can now do (require foo), (require [foo [a b c]]), (require [foo [*]]), and (require [foo :as bar]). The first and last forms get you macros named foo.a, foo.b, etc. or bar.a, bar.b, etc., respectively. The second form only gets the macros in the list.

Implements #1118 and perhaps partly addresses #277.

N.B. The new meaning of (require foo) will cause all existing code that uses macros to break. Simply replace these forms with (require [foo [*]]) to get your code working again.

There's a bit of a hack involved in the forms (require foo) and (require [foo :as bar]). When you call (foo.a ...) or (bar.a ...), Hy doesn't actually look inside modules. Instead, these (require ...) forms give the macros names that have periods in them, which happens to work fine with the way Hy finds and interprets macro calls.

If this all looks good, I'll update the documentation accordingly.

You can now do (require foo), (require [foo [a b c]]), (require [foo [*]]), and (require [foo :as bar]). The first and last forms get you macros named foo.a, foo.b, etc. or bar.a, bar.b, etc., respectively. The second form only gets the macros in the list. Implements hylang#1118 and perhaps partly addresses hylang#277. N.B. The new meaning of (require foo) will cause all existing code that uses macros to break. Simply replace these forms with (require [foo [*]]) to get your code working again. There's a bit of a hack involved in the forms (require foo) or (require [foo :as bar]). When you call (foo.a ...) or (bar.a ...), Hy doesn't actually look inside modules. Instead, these (require ...) forms give the macros names that have periods in them, which happens to work fine with the way Hy finds and interprets macro calls.

refi64 · 2016-10-27T21:35:25Z

hy/compiler.py

-            require(entry, self.module_name)
+        for entry in expression[1:]:
+            if isinstance(entry, HySymbol):
+                # e.g., (require foo)


REALLY minor nitpick, but this would be a little easier to read without the e.g., since I think it's obvious that it's an example.

refi64 · 2016-10-27T21:36:35Z

tests/resources/tlib.py



 @macro("qplah")
 def tmac(*tree):
-    return HyList(tree)
+    return HyList((HyInteger(8), ) + tree)


So the difference between the two test macros is clearer, and both actually do something, so we can catch a bug in which the macro isn't being called and its argument is returned unaltered.

refi64 · 2016-10-27T21:42:36Z

hy/compiler.py

+                __import__(entry)
+                require(entry, self.module_name, all_names=True,
+                        prefix=entry)
+            elif (isinstance(entry, HyList) and len(entry) == 2


This branch and the two that follow repeat a lot of code and don't handle stuff like (require [x [* *]]) correctly.

(1) Regarding repetition, I don't think there's enough of substance to factor out, because each branch is small but requires small differences; can you specify what kind of refactoring you'd like to see? (2) What should (require [x [* *]]) do? Import a macro named * twice? * is a terrible name for a macro.

@Kodiologist Probably be an error, though (import [x [* *]]) works for some weird reason.

refi64 · 2016-10-27T21:44:04Z

hy/compiler.py

-        for entry in expression:
-            __import__(entry)  # Import it fo' them macros.
-            require(entry, self.module_name)
+        for entry in expression[1:]:


It seems kind of odd that require and import handle a pretty much identical syntax in completely different ways. Would it somehow be possible to somehow merge the two?

They use the same syntax but do pretty different things because import actually emits code and require just changes the state of the compiler. I think trying to unify the implementations wouldn't make the compiler code much nicer.

refi64 · 2016-10-27T21:45:03Z

FWIW, I really like the idea behind this change, but this is quite literally going to one of the biggest breaking changes that have been done...

tuturto · 2016-10-29T05:55:44Z

This is good change in my opinion. It will be a bit of a pain to fix all of existing code, but it'll be worth it.

Kodiologist · 2016-10-31T16:24:40Z

@kirbyfan64 Are you happy with the changes I've made? Regarding the "e.g." in the comments, zackmdavis thumbed your comment down, and I guess I agree with him, although it's not very important in any case.

refi64 · 2016-10-31T19:16:53Z

@Kodiologist Whoops, forgot about this.

refi64 · 2016-10-31T19:17:43Z

@hylang/core Does this count as two acks now, since @tuturto said he agrees with the changes, or does someone else have to actually hit the "Approve" button?

refi64 · 2016-10-31T19:19:27Z

@Kodiologist Actually, no, I take that back: you forgot to update the docs!

Kodiologist · 2016-10-31T19:30:05Z

Okay, I'll take a stab at the docs now that everybody's happy with the feature itself.

Kodiologist · 2016-10-31T23:21:39Z

Done.

tuturto · 2016-11-01T12:44:59Z

docs/language/api.rst

-parameter specifying the module which macros should be imported. Multiple
-modules can be imported with a single ``require``.
+``require`` is used to import macros from one or more given modules. It allows
+parameters in all the same formats as ``import``. The ``require`` form itself


If I read the code correctly, following doesn't work:

(require [foo [bar :as baz]])

but similar does work for import though

(import [foo [bar :as baz]])

Embarassingly, I had no idea that from foo import bar as baz is legal Python, let alone supported in Hy. I'll add that form for require.

tuturto · 2016-11-01T12:47:11Z

docs/language/api.rst

+    (require [mymodule [*]])
+    (foo 1)
+
+Macros that call macros


This is good addition. I got bitten by this multiple times, until I learned it hard way.

refi64 · 2016-11-01T00:32:19Z

docs/language/api.rst

+parameters in all the same formats as ``import``. The ``require`` form itself
+produces no code in the final program: its effect is purely at compile-time, for
+the benefit of macro expansion. Specifically, ``require`` imports each named
+module and then makes available each requested macro in the current module.


Should this just be worded "makes each requested macro available"?

Yes, you're right.

tuturto

Looking good. Thanks for the hard work ♥

tuturto · 2016-11-02T13:01:57Z

One implication that I didn't spot, but that was pointed out to me is that if we have a module with several macros that depend on each other, we either need to make sure all needed macros are actually required. This means that client code has to require them explicitly or with *, or that the macros themselves have to require them. This can make code somewhat ugly. What do you think, how big problem this is?

Kodiologist · 2016-11-02T14:53:32Z

You're welcome. :)

if we have a module with several macros that depend on each other, we either need to make sure all needed macros are actually required. This means that client code has to require them explicitly or with *, or that the macros themselves have to require them. This can make code somewhat ugly. What do you think, how big problem this is?

That's just the same problem as I described in "Macros that call macros", right? I think having the macros themselves require their dependencies is the right thing to do, as I said in the documentation. But you can reduce repetitive code by writing a function like:

(defn require-me [&rest body]
  `(do (require mymodule) ~@body))

which you'd use like:

(defmacro foo [n]
  (require-me `(mymodule.repexpr ~n (raw-input "Gimme some input: "))))

And in general, actually, if you're writing some code specifically to be a subroutine for one or more macros, it's more convenient and less error-prone to write it as a function than a macro.

tuturto · 2016-11-03T07:35:07Z

It's just something to be aware of I guess. Since this has two acks and no open issues I'm aware of, I'll merge this into master.

refi64 requested changes Oct 27, 2016

View reviewed changes

Make require syntax stricter and add tests

a8440b7

refi64 approved these changes Oct 31, 2016

View reviewed changes

Update documentation for require

bff9640

tuturto reviewed Nov 1, 2016

View reviewed changes

refi64 approved these changes Nov 1, 2016

View reviewed changes

Kodiologist added 2 commits November 1, 2016 08:55

Documentation wording improvements

45c72ed

Allow :as in require name lists

f23c97d

tuturto approved these changes Nov 2, 2016

View reviewed changes

tuturto merged commit 14fddbe into hylang:master Nov 3, 2016

Kodiologist deleted the require-extensions branch November 3, 2016 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give `require` the same features as `import` #1142

Give `require` the same features as `import` #1142

Kodiologist commented Oct 27, 2016

refi64 Oct 27, 2016

refi64 Oct 27, 2016

Kodiologist Oct 27, 2016

refi64 Oct 27, 2016

Kodiologist Oct 27, 2016

refi64 Oct 27, 2016

refi64 Oct 27, 2016

Kodiologist Oct 27, 2016

refi64 commented Oct 27, 2016

tuturto commented Oct 29, 2016

Kodiologist commented Oct 31, 2016

refi64 commented Oct 31, 2016

refi64 commented Oct 31, 2016

refi64 commented Oct 31, 2016

Kodiologist commented Oct 31, 2016

Kodiologist commented Oct 31, 2016

tuturto Nov 1, 2016

Kodiologist Nov 1, 2016

Kodiologist Nov 1, 2016

tuturto Nov 1, 2016

refi64 Nov 1, 2016

Kodiologist Nov 1, 2016

tuturto left a comment

tuturto commented Nov 2, 2016

Kodiologist commented Nov 2, 2016

tuturto commented Nov 3, 2016

Give require the same features as import #1142

Give require the same features as import #1142

Conversation

Kodiologist commented Oct 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

refi64 commented Oct 27, 2016

tuturto commented Oct 29, 2016

Kodiologist commented Oct 31, 2016

refi64 commented Oct 31, 2016

refi64 commented Oct 31, 2016

refi64 commented Oct 31, 2016

Kodiologist commented Oct 31, 2016

Kodiologist commented Oct 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuturto left a comment

Choose a reason for hiding this comment

tuturto commented Nov 2, 2016

Kodiologist commented Nov 2, 2016

tuturto commented Nov 3, 2016

Give `require` the same features as `import` #1142

Give `require` the same features as `import` #1142