Bug #19301
closedFix Data class to report keyrest instead of rest parameters
Description
Overview¶
Hello and Happy New Year. 👋
With the new Data
class, I'm seeing a discrepancy in parameter behavior compared to a Struct
. I understand the original Data
feature request made design choices to only accept keyword arguments for the #initialize
method but reporting [[rest]]
parameters seems misleading to me because it doesn't share the more flexible Struct#initialize
behavior and would like to request Data#initialize
answer [[keyrest]]
for parameters for improved metaprogramming accuracy.
Steps to Recreate¶
To reproduce, consider the following:
DataExample = Data.define :one, :two
StructExample = Struct.new :one, :two
argument_array = [one: 1, two: 2]
argument_hash = {one: 1, two: 2}
puts "Data (parameters): #{DataExample.method(:initialize).parameters}"
puts "Struct (parameters): #{StructExample.method(:initialize).parameters}"
puts "Data (argument hash): #{DataExample[**argument_hash]}"
puts "Struct (argument array): #{StructExample[*argument_array]}"
puts "Struct (argument hash): #{StructExample[**argument_hash]}"
The above will output the following:
Data (parameters): [[:rest]]
Struct (parameters): [[:rest]]
Data (argument hash): #<data DataExample one=1, two=2>
Struct (argument array): #<struct StructExample one={:one=>1, :two=>2}, two=nil>
Struct (argument hash): #<struct StructExample one=1, two=2>
The Struct
class -- as far as I know -- has always reported [[rest]]
parameters even though it can accept positional or keyword arguments without error. ...but this is definitely not the case with the Data
class which can be seen when running the following modification to the above:
DemoExample[*argument_array]
# missing keyword: :two (ArgumentError)
The above clearly betrays the [[rest]]
parameters response (granted a Struct
is slightly devious too but at least happily accepts positional or keyword arguments). With this in mind, could Data#initalize
be fixed to at least report [[keyrest]]
so we'd have a better chance of metaprogramming the correct argument format based on the #parameters
response for initializing a Data
instance correctly?
Thanks. 🙇🏻♂️
Environment¶
ruby 3.2.0 (2022-12-25 revision a528908271) [arm64-darwin22.2.0]
Updated by zverok (Victor Shepelev) almost 2 years ago
- Related to Bug #19280: Wrong error message about arity of Data::define.new added
Updated by zverok (Victor Shepelev) almost 2 years ago
- Related to Bug #19278: Constructing subclasses of Data with positional arguments added
Updated by bkuhlmann (Brooke Kuhlmann) almost 2 years ago
Apologies, I should have used StructExample = Struct.new :one, :two, keyword_init: true
in the above example. ...but this also highlights a new behavioral change with a Struct
which doesn't use keyword_init
but misleadingly accepts positional arguments incorrectly. Should this be logged as a different issue (I'm not sure if the [[rest]]
problem has been reported yet but will look in the meantime?
Updated by zverok (Victor Shepelev) almost 2 years ago
- Status changed from Open to Rejected
See discussions in #19278 and #19280.
This is a conscious decision of Data.new
converting all arguments to keyword ones, made for ease of providing default values or preprocessing of arguments, without caring about the form they were passed in:
DataExample = Data.define(:one, :two) do
def initialize(one:, two: 0) = super(one: one.to_i, two: two.to_i)
end
DataExample.new('1')
# => #<data DataExample one=1, two=0>
DataExample.new(1)
# => #<data DataExample one=1, two=0>
DataExample.new(one: '1')
# => #<data DataExample one=1, two=0>
DataExample.new(one: '1', two: '2')
# => #<data DataExample one=1, two=2>
This would be very hard to achieve if Data
(like Struct
) would convert arguments in #initialize
.
(You can try yourself with a Struct
to define the #initialize
which does the same.)
I will work on improving the implicitness of this in the documentation, though.
I am not sure about your DemoExample
which raises (I can't see anything called DemoExample
in your definitions). This works for me, if that was meant:
DataExample[1, 2]
#=> #<data DataExample one=1, two=2>
To see (for metaprogramming, say) "all parameters that Data.new
accepts", you can do this:
DataExample.method(:new).parameters
#=> [[:rest]]
...which would be like Struct's [[:rest]]
(and not that informative, but that's typically this for C-defined methods.)
Updated by bkuhlmann (Brooke Kuhlmann) almost 2 years ago
Hey Victor, thanks. I was aware of #19278 but hadn't noticed #19280.
You are correct, when I used DemoExample[*argument_array]
earlier, that was a typo. I mean to use DataExample[*argument_array]
.
Ah, so you are saying this is a problem with C-defined methods. I hadn't realized that distinction before. Is there no way to allow C-defined methods to at least attempt to report the correct parameters because only answering [[rest]]
isn't accurate?
I suppose this goes way beyond this issue but was hoping for something kind of like this (not identical to what I'm requesting but more accurate parameter-wise):
class Demo
def self.new(**) = allocate(**)
def initialize one: 1, two: 2
super one:, two:
end
private
attr_reader :one, :two
end
puts "Demo Parameters (new): #{Demo.method(:new).parameters}"
puts "Demo Parameters (initialize): #{Demo.instance_method(:initialize).parameters}"
# Demo Parameters (new): [[:keyrest, :**]]
# Demo Parameters (initialize): [[:key, :one], [:key, :two]]
Updated by zverok (Victor Shepelev) almost 2 years ago
I am a bit lost with what you are expecting, but to clarify a bit:
-
Data.new
andData#initialize
have different signatures - If they would be defined in Ruby, the signatures would be: for
.new
:[[:rest], [:keyrest]]
, for#initialize
:[[:keyrest]]
- But C-defined methods have somewhat loose reporting on their signatures, that's why
.new
reports itself as just[[:rest]]
, I am not sure it can be made better (mnemonically, can be treated as "accepts any number of arguments, if the last of them is Hash, considers it keyword args"); anyway, it is unrelated toData
implementation. - What is reported for
Data#initialize
is correct, it really accepts only keyword args. I just published an article explaining this design decision :)
Updated by bkuhlmann (Brooke Kuhlmann) almost 2 years ago
Yeah, let me respond in order:
- Correct. No issues there.
- Correct. My problem was thinking in terms of pure Ruby, not C. That was my source of confusion, originally, but you've resolved my confusion.
- Correct. However, I think there is still an outstanding issue, architecturally, but I'll explain shortly.
- Thanks, yes, I read your article in detail and linked to it from my Ruby Data article as well.
I understand more clearly where you are coming from but am still concerned how to dynamically build a proper argument array when Struct#initialize
and Data#initialize
always return [[rest]]
for parameters. I now realize that is how C-backed objects work but that also means Method#parameters
is unreliable in terms of dealing with Struct
and Data
classes. For example, consider the following:
DataExample = Data.define :one, :two
StructAny = Struct.new :one, :two
StructKeywordOnly = Struct.new :one, :two, keyword_init: true
models = [DataExample, StructAny, StructKeywordOnly]
arguments = [{one: 1, two: 2}]
models.each do |model|
print "#{model}: "
puts model[*arguments]
rescue ArgumentError => error
puts error.message
end
# DataExample: missing keyword: :two
# StructAny: #<struct StructAny one={:one=>1, :two=>2}, two=nil>
# StructKeywordOnly: #<struct StructKeywordOnly one=1, two=2>
In all three models, asking model.method(:initialize).parameters
will always answer [[rest]]
for parameters so when I build my arguments (i.e. [{one: 1, two: 2}]
) in the same format as dictated from the Method#parameters
response then the only model that gives me the correct instance is the StructKeywordOnly
model because using keyword_init: true
does that coercion for me. 😅
I originally logged this issue because I was thinking Data
should behave more like how a Struct
works when used with keyword_init: true
since Data
enforces keyword arguments for #initialize
by default but I think this is a bigger issue than Data
alone.
Would it be better if I move this discussion to a new issue where the focus is on asking if Method#parameters
could properly answer the argument signature for C objects? Maybe that's too big of an ask?
Updated by Eregon (Benoit Daloze) almost 2 years ago
bkuhlmann (Brooke Kuhlmann) wrote in #note-7:
Would it be better if I move this discussion to a new issue where the focus is on asking if
Method#parameters
could properly answer the argument signature for C objects? Maybe that's too big of an ask?
Yes I think this would be best as a new issue, unless there is already an existing issue about this.
Updated by bkuhlmann (Brooke Kuhlmann) almost 2 years ago
Benoit, thanks. I found what I was looking for in #8088. I've added a comment to that issue referring back to this issue.
Victor, thanks for discussion on this and learning just how far down this C-based implementation problem really goes. 🙃