Skip to content

1.0 release #146

Description

@jw3126

I think the API is pretty stable and the implementation is solid. What do you think about 1.0 @tkf ? Do you have any possible breaking changes in mind, we might want to do?

Activity

  1. tkf commented on Jul 10, 2020

    @tkf
    Collaborator

    Now that you mention it, I just remembered that that there is an RFC for 2-arg get(::AbstractDict{K,V}, key) :: Union{Nothing,Some{V}} API JuliaLang/julia#34821 which would totally ruin Setfield API... 😢. Dictionary-like containers would define get(::ContainerType, key) while we need get(obj, ::Lens). This would introduce ambiguities.

    (I guess there is some chance that dict?[key] become the syntax for Union{Nothing,Some{T}} version. This would be the best-case scenario. But there is also a big chance that it'd be used for Union{T,Missing}.)

    I'm not sure what's the best strategy here. Renaming the function might be the only option but get and set are such adequate names... Is there a good name we can use instead of get? Or maybe a better name for JuliaLang/julia#34821?

  2. jw3126 commented on Jul 10, 2020

    @jw3126
    OwnerAuthor

    I actually remember pondering about whether overloading Base.get was too daring at the beginning of Setfield.jl 😄

  3. jw3126 commented on Jul 10, 2020

    @jw3126
    OwnerAuthor

    I was playing with the idea to turn this into a more radical refactoring.

    • Replace Base.get(obj, lens::MyLens) by (lens::MyLens)(obj)
    • Use much more duck typing. Get rid of the abstract Lens type. A lens can be of any type, it just needs to implement the interface
      Setfield.set(obj, lens, val) and lens(obj)
    • In this picture the emphasis is lens as a generalization of FunctionLens and not as a generalization of field access

    Pros

    • It makes defining function lenses slightly more readable: Setfield.set(obj, ::typeof(eltype), T) = ...
    • It makes a couple of nice notations more natural, e.g. @set obj |> first |> eltype = Int
    • I feel that ducktyping + interface is usally a better design then subtying + interface. It's more lightweight to not impose a supertype on downstream users.

    Cons

    • Currently we reorder compositions of more than two lenses for performance. We can't do that in the ∘ anymore.
    • In haskell lenses are realized as certain functions, so calling a lens in haskell would be a completly different operation then doing it in Setfield.
    • Pretty printing of composed lenses is a bit more tricky. But I think if Base prints f∘g as f∘g then it should be okay.
    • We would probably need to flip composition order. Currently we have:
    julia> using Setfield
    
    julia> @lens(first(_)) ∘ @lens(last(_))
    (@lens last(first(_)))

    Any thoughts @tkf ?

  4. tkf commented on Jul 11, 2020

    @tkf
    Collaborator

    I thought about lens(obj) a bit although kind of stopped considering once realized ∘ would be incompatible. But yeah, if we flip ∘, it's consistent and potentially easy to understand (like what we did in Transducers.jl). I guess it's also kind of like Clojure where the symbols are callable and do the key lookup.

    If we are going to do such a radical change, probably it's the best timing for renaming the package #54. The name Accessors.jl discussed there is also kind of compatible with the direction of the "flipped" composition.

    I also think going duck typing (+ maybe some traits at some point) is a good approach. Since we don't have multiple inheritance, we can't express all the optics hierarchy in Julian type system.

    Anyway, to be honest I'm a bit scared of this direction but I'm in favor after all. One nice thing about "owning" the API function is that we can forget about the method ambiguities like JuliaCollections/DataStructures.jl#511.

  5. goretkin commented on Jul 25, 2020

    @goretkin

    Dictionary-like containers would define get(::ContainerType, key) while we need get(obj, ::Lens). This would introduce ambiguities.

    I think I just ran into this ambiguity, but for a different reason (because of method delegation)

    using Setfield: @set!
    
    d = Dict(:k => (x=3,))
    @set! d[:k].x += 4 # works
    
    using DataStructures: DefaultDict
    
    dd = DefaultDict((x=0,), :k=>(x=3,))
    @set! dd[:k].x += 4 # does not:
    ERROR: MethodError: get(::DefaultDict{Symbol,NamedTuple{(:x,),Tuple{Int64}},NamedTuple{(:x,),Tuple{Int64}}}, ::Setfield.ComposedLens{Setfield.IndexLens{Tuple{Symbol}},Setfield.PropertyLens{:x}}) is ambiguous. Candidates:
      get(a::DefaultDict, args...) in DataStructures at /Users/goretkin/.julia/packages/DataStructures/iWRBf/src/delegate.jl:21
      get(obj, l::Setfield.ComposedLens) in Setfield at /Users/goretkin/.julia/packages/Setfield/XM37G/src/lens.jl:163
    Possible fix, define
      get(::DefaultDict, ::Setfield.ComposedLens)
    
  6. tkf commented on Jul 25, 2020

    @tkf
    Collaborator
  7. MarkusLohmayer commented on Aug 11, 2020

    @MarkusLohmayer

    Since you are talking about major refactoring and renaming of the package, maybe it should be considered how to (later) add support for sum types / prisms ...

  8. jw3126 commented on Aug 11, 2020

    @jw3126
    OwnerAuthor

    @MarkusLohmayer do you have some thoughts on this?

  9. MarkusLohmayer commented on Aug 11, 2020

    @MarkusLohmayer

    I wish I had!
    The first unknown, I guess, is the way sum types are expressed in Julia.
    According to my knowledge, Julia does not directly support algebraic datatypes like other functional languages.
    This has been argued here and here.
    So it is probably much harder to come up with an idiomatic approach to prisms.
    And secondly, to have an overall implementation of optics (lenses, prisms, ...) which can be composed with each other is also not necessarily an easy thing to do, I guess.
    I am just in the process of learning functional programming stuff. I would like to learn more and contribute, though.

  10. tkf commented on Sep 8, 2020

    @tkf
    Collaborator

    Hi, this comment is totally off-topic 😅, but this thread seems to be the best place to ping Julians who are interested in optics. FYI, it looks like Keno is looking into organizing notations for AD based on Riley, Categories of Optics (https://arxiv.org/abs/1809.00738). See PDF here https://gist.github.com/Keno/4a6507b75288b1fe671e9d1cc683014f. Not sure where the discussion is happening but I saw this in slack: https://app.slack.com/client/T68168MUP/C6G240ENA/thread/C6G240ENA-1599261433.092100

  11. jw3126 commented on Sep 8, 2020

    @jw3126
    OwnerAuthor

    Thanks @tkf . There is also the #catlab channel on slack.

  12. jw3126 commented on May 18, 2022

    @jw3126
    OwnerAuthor

    From the discussion in JuliaLang/julia#34821 it seems unlikely that it will conflict with our usage of Base.get. Setfield.jl is stable now for years, so I think we can tag 1.0. Any objections @tkf ?

  13. LilithHafner commented on Aug 9, 2022

    @LilithHafner

    Tagging a breaking release without breaking changes is not good because it requires all direct dependants to update their compact entries if they wish to continue receiving updates.

    The explanation here applies: https://discourse.julialang.org/t/please-be-mindful-of-version-bounds-and-semantic-versioning-when-tagging-your-packages/30708

    Given that at least 74 packages directly depend on Setfield, I think it is worth it to yank 1.0 from the general registry and retag master as non-breaking. See JuliaMath/NaNMath.jl#59 for a similar case.

    This is a list of 46 packages that currently depend on Setfield and do not declare support for v1.1.0
    DynamicGrids
    Ekztazy
    ExplicitFluxLayers
    Polymer
    UnionArrays
    WeightedArrays
    DataAugmentation
    BangBang
    JsonGrinder
    AeroMDAO
    SplittablesBase
    Rasters
    BenchmarkCI
    Transducers
    BifurcationInference
    MuseInference
    AlphaZero
    NaiveGAflux
    WhereTraits
    TreeKnit
    BitSAD
    ImageEdgeDetection
    Hyperopt
    FLoops
    CellMLToolkit
    AdvancedHMC
    Kaleido
    ThreadsX
    GeoData
    AbstractPPL
    HierarchicalTemporalMemory
    DynamicPPL
    FluxTraining
    ModelParameters
    CMBLensing
    TopOpt
    ReinforcementLearningExperiments
    FastAI
    MicroCollections
    Mill
    RegressionDiscontinuity
    SphericalHarmonics
    FeatureRegistries
    Cropbox
    PowerDynamics
    ReinforcementLearningZoo
    
  14. jw3126 commented on Aug 10, 2022

    @jw3126
    OwnerAuthor

    Thanks @LilithHafner for bringing this up.

    Tagging a breaking release without breaking changes is not good because it requires all direct dependants to update their compact entries if they wish to continue receiving updates.

    I agree, this is annoying. However we are talking about 1.0 here, which is a promise of stability. Personally, I am more likely to pick up a dependency that is version 1.x than the same dependency if it is 0.x. So in my view, the benefits clearly outweigh the cost here.

    I think it is worth it to yank 1.0 from the general registry

    This release is months old. Yanking it will break packages that depend on Setfield = 1, right?

  15. LilithHafner commented on Aug 10, 2022

    @LilithHafner

    You're right about yanking. I thaught it merely set a preference not to use a version but it actually makes it impossible to install (source).

    It would be nice if there was an easier way we could let the resolver know that it is okay to use setfield 1.0 anywhere 0.8 is used than making 46 PRs.

  16. jw3126 commented on Aug 10, 2022

    @jw3126
    OwnerAuthor

    It would be nice if there was an easier way we could let the resolver know that it is okay to use setfield 1.0 anywhere 0.8 is used than making 46 PRs.

    Agreed would be handy to signal that this change is not breaking. I imagine 1.0 be non-breaking is quite common. Personally, I usually release 1.0 when I did not feel the need for breaking changes for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions