Add NamedTuple#merge(other : NamedTuple) #4688

Papierkorb · 2017-07-07T22:52:58Z

Hi,

Today in #crystal-lang a user required a possibility to merge two named tuples.
Their use-case was to call methods with named arguments from inside a RESTful API, while being able to easily set default values (or force other arguments to certain values).

Sample code

# Read arguments from user
args = NamedTuple(user: String, password: String).from(rest_arg_hash)

# Force the `server` argument to a value
call_args = args.merge(server: "auth.my-service.com")

# And easily call the actual method
authenticate(**call_args)

Using this, one can easily do dynamic invocation on methods, possibly backed by further user-defined macro magic ;)

Behaviour

Takes two named tuples (The other can be kw-args style, or a proper named tuple), and merges keys and values from both into a new named tuple. If both tuples define a value for the same key, the value from the other is kept.

Cya

oprypin · 2017-07-07T23:06:25Z

src/named_tuple.cr

+  def merge(**other : **U) forall U
+    {% begin %}
+    {
+      {% for k in T %} {% unless U.keys.includes?(k) %} "{{k}}": self[:"{{k}}"],{% end %} {% end %}


Please use {{k.stringify}} and not "{{k}}". I think this is not safe.

Also, please use {{k.symbolize}} and not :"{{k}}".

need47 · 2017-07-07T23:42:16Z

I have been long waiting on this. Currently I have my own merge method. Glad to see it built-in stdlib.

Merges two named tuples into a new one. If both tuples define a value for the same key, the value of the *other* tuple is used.

Papierkorb · 2017-07-08T14:54:59Z

Now using stringify and symbolize

sdogruyol · 2017-07-09T10:44:37Z

I also had a similar case. Having a merge for NamedTuple would definitely help.

asterite · 2017-07-14T16:32:09Z

Duplicate of #2634

I think I prefer the implementation of #2634

Papierkorb · 2017-07-14T16:35:20Z

#2634 blows up if both tuples have a value with the same key.

emancu · 2017-07-15T05:11:09Z

@Papierkorb that was the main idea, I was trying to compose NamedTuples instead of merging them, so if you duplicate a key, it shouldn't compile.

In #2634 you can see a comment explaining why I discarded the merge function, however, I found really useful any implementation, so happy to see this merged :)

Papierkorb · 2017-07-15T12:13:17Z

@emancu I totally see the worth in both methods, and I'd like to see both merged.

straight-shoota · 2017-07-15T16:51:19Z

Actually I think there are three reasonable methods of combining two NamedTuples:

merge: Creates a new NamedTuple type with the union of all keys. For the same key, values from one overwrite those of the other. Different types for the same key should either fail or combine them to a union type (?). The inversion should return the same NamedTuple type with possibly different values (iff keys are intersected). Add NamedTuple#merge(other : NamedTuple) #4688
+: Creates a new NamedTuple type with the union of all keys. Fails if other and self share a key. The operation is commutative. Merge named tuples #2634
update: Creates a new instance of self's NamedTupe type with values of self updated with those of other. other's keys need to be a subset of self's keys. Inversion is only valid if the key sets are identical. The behavior is similar to merge but it ensures that no additional keys are added and preserves the types from self. Add NamedTuple#update(other : NamedTuple) to merge values but keep type #4727

~~For update there is no PR yet~~ and I don't think it has been proposed before. I am not sure if the key and type safety provided over merge adds much because mismatching types will either result in type errors elsewhere or shouldn't be to much of a concern.

Sija · 2017-07-15T17:15:37Z

@straight-shoota If I understand correctly there's no PR for update because NamedTuple is meant to be immutable.

straight-shoota · 2017-07-15T17:18:51Z

Of course it is. I'm sorry if this was misunderstanding, it would mean to create a new instance of the same NamedTuple type with updated values. I'll try to improve the wording on that.

Sija · 2017-09-22T00:01:06Z

@Papierkorb Would be sad to have this PR buried, so 🏓

asterite · 2017-09-22T11:16:18Z

@Papierkorb Could you re-format the source files? It's only failing because of that.

Sija · 2017-09-22T14:36:35Z

src/named_tuple.cr

+
+  # Returns a hash value based on this name tuple's size, keys and values.
+  #
+  # See also: `Object#hash`.


Redundant line.

Papierkorb · 2017-09-22T19:22:42Z

@asterite Ah shoot, I'm away from home this week-end. Would it be okay if I do it on Monday?

asterite · 2017-09-22T19:24:52Z

@Papierkorb Of course! Take your time :-)

Papierkorb · 2017-09-25T19:22:35Z

Like this? Or should I fixup the original commit?

oprypin reviewed Jul 7, 2017

View reviewed changes

Add NamedTuple#merge(other : NamedTuple)

9cb6620

Merges two named tuples into a new one. If both tuples define a value for the same key, the value of the *other* tuple is used.

Papierkorb force-pushed the add-namedtuple-merge branch from 87e5609 to 9cb6620 Compare July 8, 2017 12:01

RX14 approved these changes Jul 8, 2017

View reviewed changes

oprypin approved these changes Jul 8, 2017

View reviewed changes

mverzilli added kind:feature topic:stdlib labels Jul 9, 2017

asterite marked this as a duplicate of #2634 Jul 14, 2017

straight-shoota mentioned this pull request Jul 19, 2017

Add NamedTuple#update(other : NamedTuple) to merge values but keep type #4727

Closed

RX14 approved these changes Sep 22, 2017

View reviewed changes

Merge branch 'master' into add-namedtuple-merge

cf495cc

Sija reviewed Sep 22, 2017

View reviewed changes

Fix formatting in spec/std/named_tuple_spec.cr

1c6da4f

asterite approved these changes Sep 25, 2017

View reviewed changes

RX14 approved these changes Sep 25, 2017

View reviewed changes

RX14 added this to the Next milestone Sep 25, 2017

RX14 merged commit ecd3368 into crystal-lang:master Sep 25, 2017

emancu mentioned this pull request Sep 26, 2017

Merge named tuples #2634

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NamedTuple#merge(other : NamedTuple) #4688

Add NamedTuple#merge(other : NamedTuple) #4688

Papierkorb commented Jul 7, 2017

oprypin Jul 7, 2017

makenowjust Jul 8, 2017 •

edited

need47 commented Jul 7, 2017

Papierkorb commented Jul 8, 2017

sdogruyol commented Jul 9, 2017

asterite commented Jul 14, 2017

Papierkorb commented Jul 14, 2017

emancu commented Jul 15, 2017

Papierkorb commented Jul 15, 2017

straight-shoota commented Jul 15, 2017 •

edited

Sija commented Jul 15, 2017

straight-shoota commented Jul 15, 2017 •

edited

Sija commented Sep 22, 2017

asterite commented Sep 22, 2017

Sija Sep 22, 2017

Papierkorb commented Sep 22, 2017

asterite commented Sep 22, 2017

Papierkorb commented Sep 25, 2017 •

edited

Add NamedTuple#merge(other : NamedTuple) #4688

Add NamedTuple#merge(other : NamedTuple) #4688

Conversation

Papierkorb commented Jul 7, 2017

oprypin Jul 7, 2017

Choose a reason for hiding this comment

makenowjust Jul 8, 2017 • edited

Choose a reason for hiding this comment

need47 commented Jul 7, 2017

Papierkorb commented Jul 8, 2017

sdogruyol commented Jul 9, 2017

asterite commented Jul 14, 2017

Papierkorb commented Jul 14, 2017

emancu commented Jul 15, 2017

Papierkorb commented Jul 15, 2017

straight-shoota commented Jul 15, 2017 • edited

Sija commented Jul 15, 2017

straight-shoota commented Jul 15, 2017 • edited

Sija commented Sep 22, 2017

asterite commented Sep 22, 2017

Sija Sep 22, 2017

Choose a reason for hiding this comment

Papierkorb commented Sep 22, 2017

asterite commented Sep 22, 2017

Papierkorb commented Sep 25, 2017 • edited

makenowjust Jul 8, 2017 •

edited

straight-shoota commented Jul 15, 2017 •

edited

straight-shoota commented Jul 15, 2017 •

edited

Papierkorb commented Sep 25, 2017 •

edited