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

Make ScopedValues.get a method of Base.get #56939

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LilithHafner
Copy link
Member

Implements #54553

@giordano giordano requested a review from vchuravy January 2, 2025 23:03
@KristofferC
Copy link
Member

KristofferC commented Jan 7, 2025

There is no docs for what the generic concept for a two arg get is so that needs to be established first? The docs for get only list the three arg version for example.

@LilithHafner
Copy link
Member Author

Why is 2-arg get relevant to this PR?

julia> methods(Base.ScopedValues.get)
# 1 method for generic function "get" from Base.ScopedValues:
 [1] get(val::Base.ScopedValues.ScopedValue{T}) where T
     @ scopedvalues.jl:151

@KristofferC
Copy link
Member

2->1 then :P

@LilithHafner
Copy link
Member Author

LilithHafner commented Jan 7, 2025

The only 1-arg method is documented as

    get(val::ScopedValue{T})::Union{Nothing, Some{T}}

If the scoped value isn't set and doesn't have a default value,
return `nothing`. Otherwise returns `Some{T}` with the current
value.

The generic concept of a 1-arg get is

    get(x) -> Union{Some, Nothing}

If `x` has a value, return `Some` with that value, otherwise return `nothing`.

How/where would you like that documented?

@KristofferC
Copy link
Member

Just so the generic meaning shows up when one checks the help for it, just like any other method. Otherwise, you cannot extend this arity of the function since you don't know what it means.

In fact, it is preferable to just have the generic method documented, then there is no need to write an implementation specific docstring for the ScopedValue version. Could it be documented as:

"""
    get(x) -> Union{Some, Nothing}

If `x` has a value, return `Some` with that value, otherwise return `nothing`.
"""
function get(x) end

?

@KristofferC
Copy link
Member

Otherwise returns Some{T} with the current value.

Nit, should be "return"

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

Successfully merging this pull request may close these issues.

2 participants