-
Notifications
You must be signed in to change notification settings - Fork 189
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
WIP: Warn top-level pyimport during pre-compilation #634
base: master
Are you sure you want to change the base?
Conversation
location = "" | ||
end | ||
@error """ | ||
Python module `$name` imported at top-level scope$location. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized I can do what Base.depwarn
does here and pass _file
and _line
to @logmsg
. I'll do it if this entire patch makes sense.
Run function `f` while disabling warning about top-level Python | ||
imports. | ||
""" | ||
function disable_warn_pyimport_at_toplevel(f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense to warn from py"..."
etc. So this function should be renamed. Maybe disable_toplevel_warning
.
While this is attractive in principle, it's perfectly legitimate to call The right thing would be if we can issue a warning or error when any (Ideally, we could just define |
Hmm, I guess Alternatively, if we could somehow get precompilation to use our pickle serialization, then we could maybe allow precompilation with Python objects. The precompilation code uses very low-level serialization (in |
Yeah, so that's why I included const pypi = PyCall.disable_toplevel_warning() do
pyimport("math").pi
end
I was thinking to do this also with
Wow. It would be great if we can support this. |
@vtjnash, is there some way to hook into how precompilation serializes an object? I looked at |
Actually, let me take it back. A Python package may be updated after a Julia package using it is precompiled. It then could easily break pickled object when some relevant internal of the updated Python package is changed (e.g., class property is renamed). So, I think it's much safer to disallow it. |
One solution would be to run |
@stevengj no, there are generally not any hooks in there |
This PR adds a warning from
pyimport
when a user tried to import Python modules at precompile time at top-level scope. Example output:This solution probably sounds like too "pythonic" because it needs to look at
stacktrace()
and dynamically determine the scope. Butjl_generating_output
is the only overhead at run-time so I guess it's fine? Other concern is that I'm probably using a lot of non-public interface. But if it makes sense to do this in PyCall maybe we can request the public APIs to implement this PR?