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

Automatic imports can shadow the user's assignments in the REPL #791

Closed
nicolas-p opened this issue Apr 7, 2015 · 10 comments · Fixed by #1979
Closed

Automatic imports can shadow the user's assignments in the REPL #791

nicolas-p opened this issue Apr 7, 2015 · 10 comments · Fixed by #1979
Labels

Comments

@nicolas-p
Copy link
Contributor

I found a problem when I tried to use the curried version of map from the Toolz package.

In the Hy REPL:

=> (import [toolz.curried [map]]) (map add1)
<class 'map'>
=> (map add1)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
TypeError: map() must have at least two arguments.

As you can see, I can use the curried function only on the same line as the import.

IPython gives a more explicit error:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-45-c14b8be9f2cd> in <module>()
      1 from hy.core.language import map
----> 2 map(add1)

TypeError: map() must have at least two arguments.

The problem is that there is an automatic import of the original map function, which replaces the curried version that was previously imported. The same thing happens with filter.

@refi64
Copy link
Contributor

refi64 commented Apr 7, 2015

I'm slightly curious as to why you're overriding built-in Python functions. That's almost never, ever a good idea. Would it kill you to import it as cmap (for "curried map")?

@jaredly
Copy link

jaredly commented Apr 9, 2015

but I'm sure we can agree it's unexpected behavior. why should there be some "magic variables" that can't be overridden in the repl?

@refi64
Copy link
Contributor

refi64 commented May 13, 2016

Can this be closed now? This actually works in actual programs.

@gilch
Copy link
Member

gilch commented May 13, 2016

@jaredly, I agree it's surprising behavior and should be corrected. You can override built-in Python functions in Python.

>>> sum
<built-in function sum>
>>> sum = 1
>>> sum
1

@kirbyfan64: Given that Hy is a Lisp-1 without Clojure's namespaces, and without Scheme's Hygienic macros either, I can see why you think the ability to override builtins is a really bad idea:

=> (defmacro sums [&rest xs] `(sum ~xs))
=> (sums 1 2 3)
6
=> (def sum 1)
=> (sums 1 2 3)
Traceback (most recent call last):
  File "<input>", line 1, in <module>
TypeError: 'int' object is not callable
=>

But actually, Python's functions have the same issue:

>>> def sums(*xs):
...   return sum(xs)
...
>>> sums(1,2,3)
6
>>> sum = 1
>>> sums(1,2,3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 2, in sums
TypeError: 'int' object is not callable
>>>

Python doesn't have true globals, only module-level globals. Redefining builtins won't break functions using them from another module. Macros are different since they expand into code executed in the present module. This is a good argument for adding something like Clojure's namespace qualification in syntax quotes (probably using Python's module system), but it's not such a good argument against redefining builtins.

Hy also disallows reassignment of special forms:

=> (def if 1)
  File "<input>", line 1, column 6

  (def if 1)
       ^-^
HyTypeError: b"Can't assign to a builtin: `if'"


=>

But at least it gives you a clear error messsage instead of failing silently like the example in the op.

In Clojure, you don't even have to include the core (builtins) in a new namespace (module).

@refi64
Copy link
Contributor

refi64 commented May 13, 2016

@gilch Two ideas:

  • "Magic" imports could add a check first:

    # before
    from hy.core.language import map
    
    # now
    try:
        map
    except NameError:
        from hy.core.language import map
  1. My preferred solution: import everything from hy.core.language on REPL start and tell the compiler to not insert any imports in the REPL. Remember, this is only a problem in the REPL; in normal scripts, the functions are imported at the top of the file, so any other imports override them anyway.

@gilch
Copy link
Member

gilch commented May 14, 2016

Does Hy always import everything in every script, or only the things it needs for that script? hy2py doesn't, but I'm not certain about Hy internally. Would this noticeably impact repl startup time? Even on a mobile device?

If we do do everything up front, then hy --spy won't show the imports happening? Or will it only show from hy.core.language *? This seems less useful for learning about how Hy works than your first idea.

I wonder if there's a way to make hy.core.language work more like Python's __builtins__?

@gilch
Copy link
Member

gilch commented May 14, 2016

I'm also concerned about how this might affect Hy's ability to support doctests #1019. We might need to be able to eval (exec?) Hy code from strings to make this work.

@Kodiologist
Copy link
Member

Paul pointed out that you can hit this with a standard Python module, string:

=> (import string)
=> string.digits
AttributeError: 'function' object has no attribute 'digits'

@gilch
Copy link
Member

gilch commented Aug 1, 2017

Python's builtins are in the builtins module. This module is mutable, though altering it is frowned upon. If we put all of Hy's core into builtins, then they would be available in every module. There's a lot to like about this approach. It would fix this issue, for one thing. It would remove the weird autoimports in the compiler. It could make macros more reliable. It would also make Hy's core more discoverable, since you could find everything in __builtins__ with dir. It's an option that we should seriously consider.

But I'm reluctant to do it that way. The problem is that importing hy from Python will pollute the builtins module for all other modules, even those written in Python. These are true globals, with all their problems. Projects that do this can interfere with each other if they try to set builtins with the same name. That's why most don't, and those that do get well-justified complaints.

One way to mitigate the damage would be to use Clojure-style namespacing of symbols. #277 We'd then only put HySymbols containing a / into builtins, which would be unlikely to interfere with any other Python use of the builtins module, since these are valid dict keys, but not valid Python identifiers. If you want the shorter alias without the /, then you simply def a shorter alias. We could even make a macro to automate this for a given namespace e.g. (using core) would def all core/foo symbols in builtins to foo in the current module (for whatever foo). But we'd write our macros to always use the namespaced / form so we don't have to depend on this.

It's a real option.

But there might be a way to create a hy_builtins module that works in Hy modules like builtins does for Python modules. Then we wouldn't have to have it at all on the Python side. I'm not exactly sure how Python modules are able to delegate to builtins when their own dict doesn't have a requested identifier. Maybe we could do it the same way.

Suppose if all Hy modules had a custom dict implementation. It could override __getitem__ to delegate to hy_builtins whenever a symbol wasn't found in the module. One possible approach might be to subclass ModuleType to use this custom dict, and then use import hooks to make sure all loaded Hy modules are of this new type, before they are executed. I'm not sure how best to do this, especially considering how much import hooks have evolved over the Python versions we're still supporting.

I was considering something like this anyway for dynamic variables hylang/hyrule#51. With dynamic variables we could even have the *ns* var and implement something like Clojure's namespace macros and the cool features that come with it, like the auto-expansion of ~ to use namespaced symbols and ::foo for auto-namespaced keywords.

@Kodiologist Kodiologist changed the title Automatic import of map and filter (and maybe others?) Automatic imports can shadow the user's assignments in the REPL Jun 21, 2019
@Kodiologist
Copy link
Member

The previously mentioned functions (e.g., map, filter, string) have been removed from Hy core, but you can observe this issue with any core function, such as drop, with input such as:

=> (setv drop 1)
=> drop
<function drop at 0x7fa40eb3b950>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants