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

Breaks down on parametric types #39

Open
nielsls opened this issue Dec 13, 2021 · 8 comments
Open

Breaks down on parametric types #39

nielsls opened this issue Dec 13, 2021 · 8 comments

Comments

@nielsls
Copy link

nielsls commented Dec 13, 2021

In Accessors v0.1.5.

Breaks down when the type parameter is not "used" in the struct.

julia> using Accessors

julia> struct MyType{T}
           i::Int64
       end

julia> m = MyType{String}(42)
MyType{String}(42)

julia> @set m.i += 1
ERROR: MethodError: no method matching MyType(::Int64)
Stacktrace:
 [1] setproperties_object(obj::MyType{String}, patch::NamedTuple{(:i,), Tuple{Int64}})
   @ ConstructionBase C:\Users\nls\.julia\packages\ConstructionBase\N3vfl\src\ConstructionBase.jl:149
 [2] setproperties(obj::MyType{String}, patch::NamedTuple{(:i,), Tuple{Int64}})
   @ ConstructionBase C:\Users\nls\.julia\packages\ConstructionBase\N3vfl\src\ConstructionBase.jl:63
 [3] set
   @ C:\Users\nls\.julia\packages\Accessors\pI3fw\src\optics.jl:376 [inlined]
 [4] _modify
   @ C:\Users\nls\.julia\packages\Accessors\pI3fw\src\optics.jl:200 [inlined]
 [5] modify(f::Accessors._UpdateOp{typeof(+), Int64}, obj::MyType{String}, optic::Accessors.PropertyLens{:i})
   @ Accessors C:\Users\nls\.julia\packages\Accessors\pI3fw\src\optics.jl:180
 [6] top-level scope
   @ REPL[4]:1
@rafaqz
Copy link
Member

rafaqz commented Dec 13, 2021

You need to define ConstructionBase.constructorof for any type where the parameters are not directly related to the fields.

@nielsls
Copy link
Author

nielsls commented Dec 14, 2021

But would it be impossible for the example above to "just" work out-of-the-box?

@jw3126
Copy link
Member

jw3126 commented Dec 14, 2021

But would it be impossible for the example above to "just" work out-of-the-box?

It is possible, but we like it simple stupid 😄 see also JuliaObjects/ConstructionBase.jl#27

@amontoison
Copy link

amontoison commented Jan 20, 2024

Is it possible to provide an example to how we should define ContructionBase.constructorof with the following structure?

struct ugo_control_type{T}
  error::Cint
  out::Cint
  print_level::Cint
  start_print::Cint
  stop_print::Cint
  print_gap::Cint
  maxit::Cint
  initial_points::Cint
  storage_increment::Cint
  buffer::Cint
  lipschitz_estimate_used::Cint
  next_interval_selection::Cint
  refine_with_newton::Cint
  alive_unit::Cint
  alive_file::NTuple{31,Cchar}
  stop_length::T
  small_g_for_newton::T
  small_g::T
  obj_sufficient::T
  global_lipschitz_constant::T
  reliability_parameter::T
  lipschitz_lower_bound::T
  cpu_time_limit::T
  clock_time_limit::T
  second_derivative_available::Bool
  space_critical::Bool
  deallocate_error_fatal::Bool
  prefix::NTuple{31,Cchar}

  ugo_control_type{T}(error::Cint, out::Cint, print_level::Cint, start_print::Cint,
    stop_print::Cint, print_gap::Cint, maxit::Cint, initial_points::Cint, storage_increment::Cint,
    buffer::Cint, lipschitz_estimate_used::Cint, next_interval_selection::Cint, refine_with_newton::Cint, alive_unit::Cint,
    alive_file::NTuple{31,Cchar}, stop_length::T, small_g_for_newton::T, small_g::T, obj_sufficient::T, global_lipschitz_constant::T,
    reliability_parameter::T, lipschitz_lower_bound::T, cpu_time_limit::T, clock_time_limit::T, second_derivative_available::Bool,
    space_critical::Bool, deallocate_error_fatal::Bool, prefix::NTuple{31,Cchar}) where T = new(error, out, print_level, start_print, stop_print,
      print_gap, maxit, initial_points, storage_increment, buffer, lipschitz_estimate_used, next_interval_selection, refine_with_newton,
      alive_unit, alive_file, stop_length, small_g_for_newton, small_g, obj_sufficient, global_lipschitz_constant, reliability_parameter,
      lipschitz_lower_bound, cpu_time_limit, clock_time_limit, second_derivative_available, space_critical, deallocate_error_fatal,
      prefix)

  ugo_control_type{T}() where T = new()
end

using Accessors
control = ugo_control_type{Float64}()
@reset control.print_level = Cint(1)

The parameter T is directly related to the fields but it's not working.

MethodError: no method matching ugo_control_type(::Int32, ::Int32, ::Int64, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::Int32, ::NTuple{31, Int8}, ::Float64, ::Float64, ::Float64, ::Float64, ::Float64, ::Float64, ::Float64, ::Float64, ::Float64, ::Bool, ::Bool, ::Bool, ::NTuple{31, Int8})
Stacktrace:
 [1] setproperties_object(obj::ugo_control_type{Float64}, patch::@NamedTuple{print_level::Int64})
   @ ConstructionBase ~/.julia/packages/ConstructionBase/JFKjZ/src/ConstructionBase.jl:217
 [2] setproperties(obj::ugo_control_type{Float64}, patch::@NamedTuple{print_level::Int64})
   @ ConstructionBase ~/.julia/packages/ConstructionBase/JFKjZ/src/ConstructionBase.jl:131
 [3] set(obj::ugo_control_type{Float64}, l::PropertyLens{:print_level}, val::Int64)
   @ Accessors ~/.julia/packages/Accessors/hjkBO/src/optics.jl:375
 [4] top-level scope
   @ REPL[9]:1

@amontoison
Copy link

Update: One solution is to define this alternative constructor:

  ugo_control_type(error::Cint, out::Cint, print_level::Cint, start_print::Cint,
    stop_print::Cint, print_gap::Cint, maxit::Cint, initial_points::Cint, storage_increment::Cint,
    buffer::Cint, lipschitz_estimate_used::Cint, next_interval_selection::Cint, refine_with_newton::Cint, alive_unit::Cint,
    alive_file::NTuple{31,Cchar}, stop_length::T, small_g_for_newton::T, small_g::T, obj_sufficient::T, global_lipschitz_constant::T,
    reliability_parameter::T, lipschitz_lower_bound::T, cpu_time_limit::T, clock_time_limit::T, second_derivative_available::Bool,
    space_critical::Bool, deallocate_error_fatal::Bool, prefix::NTuple{31,Cchar}) where T = new{T}(error, out, print_level, start_print, stop_print,
      print_gap, maxit, initial_points, storage_increment, buffer, lipschitz_estimate_used, next_interval_selection, refine_with_newton,
      alive_unit, alive_file, stop_length, small_g_for_newton, small_g, obj_sufficient, global_lipschitz_constant, reliability_parameter,
      lipschitz_lower_bound, cpu_time_limit, clock_time_limit, second_derivative_available, space_critical, deallocate_error_fatal,
      prefix)

@aplavin
Copy link
Member

aplavin commented Jan 20, 2024

Do you really need the inner constructor defined manually?

The condition required for everything to work automatically is that your type can be created from its field values, without manually specifying type parameters. In your case, it corresponds to ugo_control_type(error, out, print_level, ...) working.
If that's not the case, constructorof definition is necessary.

@amontoison
Copy link

Yes, I need the inner constructor defined manually because ugo_control_type{T}() where T = new() is needed and it overwrites the default one.

A C routine will set the default values of this structure so control = ugo_control_type{T}() is user-friendly to have something similar to the C definition

ugo_control_type control

But I find a way to add automatically the "long" constructors when I generate the structures with Clang.jl.

An example of contructorof in the documentation / README could be nice 👍

@aplavin
Copy link
Member

aplavin commented Jan 21, 2024

An example of contructorof in the documentation / README could be nice 👍

Yeah, I guess https://juliaobjects.github.io/ConstructionBase.jl/ doesn't give all the details needed to explain constructorof from the start... Feel free to suggest doc improvements there!

Yes, I need the inner constructor defined manually because ugo_control_type{T}() where T = new() is needed and it overwrites the default one.

Btw, there's a (poorly documented) way to create a struct like new() does, but without a custom inner constructor. Just add this outer constructor:

@generated ugo_control_type{T}() where {T} = Expr(:new, ugo_control_type{T})

Unlike inner constructors, it doesn't overwrite the default one, so everything should work without all that repetition of fieldnames.

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

No branches or pull requests

5 participants