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

@register ambiguity #788

Closed
ArnoStrouwen opened this issue Feb 19, 2024 · 10 comments
Closed

@register ambiguity #788

ArnoStrouwen opened this issue Feb 19, 2024 · 10 comments
Labels

Comments

@ArnoStrouwen
Copy link
Member

Strange ambiguity pops up, even though @register is nowhere used.
I tried the same in Symbolics, SymbolicUtils and ModelingToolkit and did not find the same behavior there.

julia> using Catalyst
julia> using Test
julia> Test.detect_ambiguities(Catalyst)
WARNING: using deprecated binding Symbolics.@register in Catalyst.
, use @register_symbolic instead.
Catalyst.@register is deprecated, use Symbolics.@register_symbolic instead.
ERROR: use of deprecated variable: Catalyst.@register
Stacktrace:
 [1] detect_ambiguities(mods::Module; recursive::Bool, ambiguous_bottom::Bool, allowed_undefineds::Any)
   @ Test C:\Users\arno\.julia\juliaup\julia-1.10.0+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Test\src\Test.jl:1968
 [2] detect_ambiguities(mods::Module)
   @ Test C:\Users\arno\.julia\juliaup\julia-1.10.0+0.x64.w64.mingw32\share\julia\stdlib\v1.10\Test\src\Test.jl:1921
 [3] top-level scope
   @ REPL[4]:1

I do not understand how it gets past:
https://github.com/JuliaLang/julia/blob/master/stdlib/Test/src/Test.jl#L2078
Since:

julia> Base.isdeprecated(Catalyst, Symbol("@register"))
true
@isaacsas
Copy link
Member

Is there something actionable here? Searching on Github I don't see that we use @register anywhere?

@ArnoStrouwen
Copy link
Member Author

Does not seem actionable to me, but not sure where else to report it. I'm not certain this is a Julia bug.

@isaacsas
Copy link
Member

I don’t get this warning in a clean environment with the latest Catalyst on 1.10. Are you using an older version?

@ArnoStrouwen
Copy link
Member Author

Yes, I'm on the latest. Did you run with julia --depwarn=error?
The end goal is to switch that on for all of SciML testing.

@ChrisRackauckas
Copy link
Member

Has it been a deprecation long enough to just make it error now? A release with that as an error would be a clear trigger.

@ArnoStrouwen
Copy link
Member Author

CI confirms the issue, @isaacsas:
https://github.com/SciML/Catalyst.jl/actions/runs/8004885702/job/21863127546?pr=789#step:6:721

@ChrisRackauckas The issue is that the deprecated binding is still part of the names in the modules (as it should be):
https://github.com/JuliaLang/julia/blob/master/stdlib/Test/src/Test.jl#L2077
When running Julia with -depwarn=errors this then turns a warning into an error here:
https://github.com/JuliaLang/julia/blob/master/stdlib/Test/src/Test.jl#L2087
However, you would expect the function to never get to that line, because of
https://github.com/JuliaLang/julia/blob/master/stdlib/Test/src/Test.jl#L2078
Indeed, this is how it's working in e.g. ModelingToolkit.
And also when I mimic this line in the REPL it gives the desired result.
I do not understand enough about Base to investigate further than this.

@ChrisRackauckas
Copy link
Member

Should we just remove the deprecation path then?

@isaacsas
Copy link
Member

Very weird, thanks for checking!

@isaacsas
Copy link
Member

If there is a change we need to make here I'm happy to, but have no idea what is causing this (in Catalyst).

@ArnoStrouwen
Copy link
Member Author

JuliaSymbolics/Symbolics.jl#1071 solves this.

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

No branches or pull requests

3 participants