-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Breaking change in module variable assignment #56933
Comments
Regarding @Keno's comment in the commit from #54678
Could we maybe try doing a softer |
First, sorry that I missed your packages when I went around fixing this in the package ecosystem. On a side note, I would also strongly advise reconsidering the automatic introduction of globals in Julia modules from the python side also. Following the 1.12 binding age partition (which has not landed yet), your users will likely run into a significant number of world age confusions. I understand that python users are used to being able to do this kind of thing, but it's not idiomatic in Julia. |
Thanks for the reply and for acknowledging the oversight. I appreciate the work that went into fixing packages in the ecosystem. My main concern about how the change was implemented: The current error message makes no mention that this syntax used to work and was intentionally changed. This is problematic because:
My proposed solution is either:
Regarding PkgEval and GitHub-based fixes, I quote Goodhart's law: "When a measure becomes a target, it ceases to be a good measure." While PkgEval and GitHub are valuable for exploring stability of new updates, and helping people make patches, it shouldn't be relied on as a primary metric for evaluating breaking changes. Proprietary Julia libraries in production environments and industry settings will never appear in PkgEval – that code will just break. End-user code - written by people who are just using Julia as a day-to-day programming language – will just break. Breaking changes need to consider this broader ecosystem. 1 year is a long time for software development so I really think it needs more warning. Not to mention some people will stick to LTS Julia for years before upgrading. (Regarding future changes to globals and world age, thanks for pointing this out – when you implement these changes, please provide warning messages for at least a few versions beforehand if possible. Breaking changes are inevitable, but they should come with clear guidance on how to update code.) Regarding the Python package in particular - We have a patch ready for PythonCall.jl (JuliaPy/PythonCall.jl#583), but I think it's important to provide some context on why we plan to keep it on the Python user side: this type of assignment has been fundamental to the Python<->Julia ecosystem for years, long before my time, and before it was apparently added to Julia itself. This syntax even appears on the quickstart page of pyjulia by @tkf (pyjulia is 8+ years old though I'm not sure when this feature was added): While pyjulia implemented this functionality manually and didn't rely on Julia's module variable assignment directly, newer packages like juliacall used this syntax when it became available in Julia – which is why it broke code for the ecosystem. (And juliacall has the unfortunate feature of automatically updating to the newest Julia version (which we should probably change) – meaning code is currently breaking without the user even changing anything) |
As requested in #56933, acknowledge that this error was missing in Julia 1.9 and 1.10 and provide a reference to the issue so that people have an easier starting point.
I completely understand the frustration, but these things are tough calls. We're not perfect, so mistakes happen. In every release there are probable two or three dozen error paths that get added or deleted that were accidentally present or absent. Those cases are then a question of pragmatism. In some cases, such as this one, the mistake is problematic enough that it needs to be immediately corrected. In others, it's annoying but not serious so we leave it. We do use PkgEval to assess the situation as part of that pragmatic evaluation. We do in general need better mechanisms to handle these kinds of situations (e.g. #54903 and related, although we may not have used it in this situation) - our practice in this regard hasn't really changed, but the ecosystem has gotten a lot larger, so the frequency of it causing pain for our users has increased. I've submitted #56956 to adjust the error message to leave a reference. Again, I don't think we can change this behavior on 1.11 retroactively. At most, we could add the deprecation on 1.10, but our historical practice, has not been to not backport such things. |
Thanks, understandable. The error message is a good start - I will leave comments on it to suggest some tweaks. Pragma strict sounds awesome by the way. You should check out the API in DispatchDoctor.jl for a local scoping approach to strictness which could be borrowed. |
I'd like to raise a concern about what appears to be a breaking change in Julia 1.11 regarding module variable assignment syntax:
While I understand from #54607 that having to declare
Main.eval(Expr(:global, :x))
is now required, this change has caused immediate downstream impacts in the ecosystem. For example, PythonCall.jl/juliacall is experiencing breaks: JuliaPy/PythonCall.jl#582. I have written various educational material in PyJulia/juliacall that have relied on this syntax (where thesetproperty!
acts as a translation layer to Julia bindings) and those are now broken. This was also used as a mechanism to declare Python functions in Julia – JuliaPy/PythonCall.jl#394Given that this modifies long-standing and very simple behavior that is used often in scripts (which are unlikely to show up in PkgEval), would it be possible to reconsider this as a breaking change more appropriate for a major version bump (i.e., Julia 2.0), or maybe do a soft deprecation for a few minor versions to give users warning?
The text was updated successfully, but these errors were encountered: