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

Inconsistent behaviour of hasproperty with Modules #47150

Open
tecosaur opened this issue Oct 13, 2022 · 8 comments
Open

Inconsistent behaviour of hasproperty with Modules #47150

tecosaur opened this issue Oct 13, 2022 · 8 comments
Labels

Comments

@tecosaur
Copy link
Contributor

Julia 1.8.2, Linux

I feel like the behavior of {has,get}{property,field} is a bit suspect when it comes to Modules. The following example should be sufficiently illustrative.

julia> hasproperty(Main, :DataFrames)
false

julia> getproperty(Main, :DataFrames)
DataFrames

julia> hasfield(Main, :DataFrames)
ERROR: MethodError: no method matching hasfield(::Module, ::Symbol)
Closest candidates are:
  hasfield(::Type, ::Symbol) at reflection.jl:215
Stacktrace:
 [1] top-level scope
   @ REPL[26]:1

julia> getfield(Main, :DataFrames)
DataFrames
@jishnub
Copy link
Contributor

jishnub commented Oct 13, 2022

Related: #46857

@KristofferC
Copy link
Member

For modules, I think it makes more sense to use isdefined.

@oxinabox
Copy link
Contributor

I feel liked getfield has always been a bit of a pun when used on modules.
But while we support it, it kins of seems like we should complete the metaphor with the others from
{has,get}{property,field}
Or deprecate it and provide an alternative

@simeonschaub
Copy link
Member

Or deprecate it and provide an alternative

That's basically what #44231 did and why we now have getglobal. We don't really want to add a loud deprecation warning though since it's too commonly used and the benefits of the change aren't worth the annoyance of a ton of warnings

@oxinabox
Copy link
Contributor

oxinabox commented May 1, 2023

Makes sense. We really need to sort out deprecating warnings that warn only for deprecations that are directly your fault.

IMO we can close this as resolved

@ericphanson
Copy link
Contributor

I don't think this is resolved since getglobal only replaces getfield. The docstring for getglobal recommends getproperty, so getproperty definitely doesn't seem to be deprecated, but getproperty is still in contradiction to hasproperty for modules, so there is still an issue.

@ericphanson
Copy link
Contributor

it is valid to add hasproperty(mod::Module, x::Symbol) = isdefined(mod, x) ? That could at least bring hasproperty and getproperty into alignment.

Or should getproperty be deprecated for modules also? (e.g. at least remove it from the getglobal docstring)

@vtjnash
Copy link
Member

vtjnash commented May 26, 2024

Seems valid, though I think it may be isresolved and ispublic?

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

8 participants