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.

Sign up for GitHub

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

Jump to bottom

hasproperty() analogue for optics #92

Open
staticfloat opened this issue Mar 21, 2023 · 3 comments
Open

hasproperty() analogue for optics #92

staticfloat opened this issue Mar 21, 2023 · 3 comments

Comments

@staticfloat
Copy link

staticfloat commented Mar 21, 2023 •
edited
Loading

Sometimes I'd like to check beforehand whether an optic is truly applicable to a type; especially when dealing with deeply-nested optics. It would be quite useful if there existed a hasproperty() analogue for optics on a type. I've made a simple version for PropertyLens and ComposedFunction, but I'm not sure about the rest of the optic types. I'm opening this issue so that if someone else is interested, they can carry this to completion.

function Base.hasproperty(x, o::PropertyLens)
    return typeof(o).parameters[1] ∈ propertynames(x)
end
function Base.hasproperty(x, o::ComposedFunction)
    return hasproperty(x, o.outer) &&
           hasproperty(o.outer(x), o.inner)
end

This allows tests such as the following:

using Accessors

struct T
    a
    b
end

x = T(T(1,2), 3)

@show hasproperty(x, @optic _.a)
@show hasproperty(x, @optic _.b)
@show hasproperty(x, @optic _.c)
@show hasproperty(x, @optic _.a.a)
@show hasproperty(x, @optic _.a.a.a)
@show hasproperty(x, @optic _.b.a)
@jw3126
Copy link
Member

jw3126 commented Mar 25, 2023

Thanks @staticfloat ! Would be nice to have such functionality. I thing it should however not be a method of Base.hasproperty. One reason is that it can be generalized to handle other lenses like IndexLens and then the name makes less sense. The other reason is that overloads like Base.hasproperty(x, o::ComposedFunction) are type piracy.

@aplavin
Copy link
Member

aplavin commented Apr 23, 2023

This function is now available as hasoptic(obj, optic)::Bool in AccessorsExtra.
Also, see maybe(optic) there: it's often more convenient than manually handling has/hasn't cases.
I think more real-world usage and testing of the interface is needed before inclusion in Accessors proper (or it shouldn't be included here at all).

@aplavin
Copy link
Member

aplavin commented Aug 10, 2023 •
edited
Loading

I've been using maybe-optics for some time already, and quite like them. They are based on hasoptic check, through I didn't really find myself using this check explicitly.
Would be nice to include into Accessors proper at some point, but there are a few questions on the semantics of hasoptic and of maybe. Here's a list of concerns about the "get" part alone:

  • maybe(o)(x) returns nothing when !hasoptic(x, o). So, composability requires hasoptic(::Nothing, _) = false to be able to combine multiple maybe optics. Presumably it's fine to use nothing in this role of sentinel, and I didn't experience any issues with that - but technically it is a limitation for when someone wants to actually operate on nothing. I don't see other alternatives, but may be missing something.
  • It's clear what hasoptic should do on specific optics like PropertyLens, IndexLens and similar. But what to do in the general case?
    • Not define hasoptic(_, _) fallback? Then any optic without a specific hasoptic method wouldn't work with hasoptic/maybe, and the majority of optics won't have it.
    • Define hasoptic(_, _) = true? That's how it's done in AccessorsExtra for now. One can put arbitrary optics into maybe, like maybe(@optic _.a + 1). However, there are cases when hasoptic(x, o) == true but o(x) fails - so maybe(o)(x) does as well. The majority of optics, both in Accessors and user-defined, presumably work on all instances of corresponding types, so true is a reasonable fallback imo.
  • How should hasoptic/maybe treat multivalued optics like Elements() and Properties()? Currently, it just errors.

@aplavin aplavin mentioned this issue Apr 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@staticfloat @aplavin @jw3126