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

Use of Base.MainInclude.eval no longer works on 1.11 #54057

Closed
MilesCranmer opened this issue Apr 11, 2024 · 6 comments
Closed

Use of Base.MainInclude.eval no longer works on 1.11 #54057

MilesCranmer opened this issue Apr 11, 2024 · 6 comments

Comments

@MilesCranmer
Copy link
Member

There is the function Base.MainInclude.eval which is publicly documented here: https://docs.julialang.org/en/v1.10/base/base/#Base.MainInclude.eval

Screenshot 2024-04-11 at 21 59 31

However on 1.11 it seems this function no longer shows up in the docs, and does not seem to work:

julia> Base.MainInclude.eval(quote println("Hello") end)
ERROR: MethodError: no method matching eval(::Expr)
You may have intended to import Base.eval
The function `eval` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  eval(::Module, ::Any)
   @ Core boot.jl:428

Stacktrace:
 [1] top-level scope
   @ REPL[7]:1

whereas on 1.10:

julia> Base.MainInclude.eval(quote println("Hello") end)
Hello

This is breaking some CI in SymbolicRegression.jl (used here). Was this function removed by mistake? Maybe I am misinterpreting what part of the API is stable?

I see this PR from @JeffBezanson which might be relevant: #51878

Thanks,
Miles

@Keno
Copy link
Member

Keno commented Apr 12, 2024

The documentation printing is a bit confusing there. This documents the Main.eval helper (which still works on 1.11), the MainInclude part is an unstable implementation detail, but regardless, use Core.eval(Core.Main, x) directly rather than reaching for the interactive helper, or even better, stop using Main.

@Keno Keno closed this as completed Apr 14, 2024
@MilesCranmer
Copy link
Member Author

Could 1.11 alias Base.MainInclude.eval for temporary backwards compatibility? Just for for versions 1.11 and 1.12 with a deprecation warning pointing to the updated version.

Since it is exported and documented I don’t think it’s good to remove it without any warning. Just having a simple alias for a couple versions is a good compromise — this is what Python does when they make a breaking change within 3.x. For example parts of the Python stdlib importlib had breaking changes, and they gave us a deprecation warning from versions 3.4 until 3.12 when they finally removed it.

It’s very helpful for developers so we have some notice to adapt.

@Keno
Copy link
Member

Keno commented Apr 14, 2024

Deprecation warnings aren't active currently, so this would be the only thing with one. This the Base.MainInclude part of this was also never part of the public API - it was a hack to make the eval import weak. The fact that that path showed up in the docs is a bit of a bug. The thing being documented (the public API), as described in the docstring, is the ability to write eval(x) in any module and have that be equivalent to Core.include(@__MODULE__, x). That still works, even in Main. The only thing that changed are some internals. I see 4-5 uses total across the ecosystem. I don't see a compelling reason to come up with a custom deprecation mechanism just for this, and while I don't think there's a problem with continuing to provide the symbol, if we do that without a deprecation mechanism, it'll just be the same situation a couple releases down the line.

@MilesCranmer
Copy link
Member Author

Okay thanks for the response.

I see 4-5 uses total across the ecosystem.

Just to mention — I really don’t think this is a good way to make decisions on breaking the public API. If someone is using Julia internally at their company, with their own internal registry, their code will not be publicly indexed. Who knows if someone has been making extensive use of some tiny part of the API that they assumed was public and stable, and propagated across their internal ecosystem. It’s just not a good policy to be inconsistent on this stuff.

I think having a consistent rule is the only option, like if the public API changes (even if it was mistakenly listed as part of the public API), then an alias should be created with a warning for the next few minor releases or something.

Anyways this doesn’t affect my code as I’ve already fixed it, but just leaving my thoughts for posterity as I really want Julia to get more traction in industry.

@Keno
Copy link
Member

Keno commented Apr 14, 2024

Again, it's not part of the public API. I understand why it may look that way, but the public API is eval(x) as mentioned in the docstring. The module of origin is just extra metadata that documenter picks up that looks confusing in this case. So the correct question here is if this implementation detail is widely used enough to warrant special consideration, which is why I looked at usage in the ecosystem.

@MilesCranmer
Copy link
Member Author

Thanks. (Just to be clear I’m playing Devil’s advocate here a bit; my needs have been met!)

Again, it's not part of the public API. I understand why it may look that way, but the public API is eval(x) as mentioned in the docstring. The module of origin is just extra metadata that documenter picks up that looks confusing in this case.

How could we know this though, without being a core dev? I think even if it’s listed by mistake as part of the public API — as it has been for several years — it should be treated as such.

So the correct question here is if this implementation detail is widely used enough to warrant special consideration, which is why I looked at usage in the ecosystem.

Again, there’s no way to know the actual usage in the ecosystem. All we have is the open-source viewpoint. This might be the majority of Julia code now, but as a convention, I don’t think it’s good to ignore closed-source projects in any pH test of potentially disruptive decisions.

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

No branches or pull requests

2 participants