Add support for Union type in structured configs. #381

pereman2 · 2020-09-20T16:15:04Z

To add support for UnionTypes a new type of node is created UnionNode. This node holds a value which is another node so it's easier to check values. I plan to modify _validate_value which validates that the input is correct with the values implementation of validate_and_convert because UnionNode should not know what a ListConfig or DictConfig is.

I added some tests to check the behaviour. At this time I don't know which ones I missed, I'll check later.

Still have to check optional unions like Optional[Union[str, int]] and I will change when pickling unions on 3.6 still fails.

Tests list

A list of use cases to cover with tests, add more here if you think of more.

Unions with optional:

Optional[Union[int, str]]
Union[int,str,None] # I think this is legal to define Optional

Union with containers, Lists, Dicts
Union[str, List[str]]
Containers with Unions:
List[Union[int, str]]
Assignment to union fields, make sure it respects the type and that the field is still a UnionNode
Merging into Union fields, , make sure it respects the type and that the field is still a UnionNode
As you pointed out, pickling with Union nodes.

Closes #144 .

omry · 2020-09-20T18:18:22Z

Cool.
A few surface area points to test/consider:

Unions with optional:

Optional[Union[int, str]]
Union[int,str,None] # I think this is legal to define Optional

Union with containers, Lists, Dicts
Union[str, List[str]]
Containers with Unions:
List[Union[int, str]]
Assignment to union fields, make sure it respects the type and that the field is still a UnionNode
Merging into Union fields, , make sure it respects the type and that the field is still a UnionNode
As you pointed out, pickling with Union nodes.

omegaconf/_utils.py

omegaconf/nodes.py

omegaconf/_utils.py

ghost · 2020-11-13T17:48:07Z

Congratulations 🎉. DeepCode analyzed your code in 6.138 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

pereman2 · 2020-11-13T17:51:27Z

I've done several things. I've removed validation with other functions since as you pointed out, it doesn't make much sense.
I've done Containers with Union element_types with more or less test. The same goes to Optional and merge with union node.

I'll try to add more test cases and wait for your review because I think you might want to comment a lot.

Still pickle not done.

omry · 2020-11-14T18:47:03Z

Re-request review when you want me to review again.

pereman2 · 2020-11-17T17:52:06Z

Hey! I'll comment every change by the tasks:

Union with containers, Lists, Dicts
Union[str, List[str]]

I've removed the function (662772b) to check whether the value is a valid type and changed it so it's checked when wraping the value with each element_type(until it finds a valid one, if not, it raises error).

Containers with Unions: (662772b)
List[Union[int, str]]

In this case it didn't need that much work. I you add a new value to a DictConfig or ListConfig it will wrap with type_ Union and union will handle the rest.

Optional[Union[int, str]] (19a4012)

Optional[Union[int ,str]] returns Union[int, str, NoneType] directly so if none is inside Union, UnionNode's optional will be true then, it removes None from Union[int, str] because optional it's checked in Node's validate.

Assignment to union fields, make sure it respects the type and that the field is still a UnionNode

I added test for this one in 9037ea0 and the tests passed.

Merge with UnionNodes (9037ea0)

After adding tests here I foound that I didn't test structured config assignment to union nodes and I had to add specific validation for it. Moreover since I use wrap node to check if a value is correct I didn't want to wrap Nodes again(it will return the node and not check) so,I used the value of the node to wrap.

Pickle with UnionNodes.

The way I faced this is transforming Union[vt1, vt2] to the list of element_types and then when loading I use the list to reconstruct the Union type as before. It looks strange but for example: u = Union[str, int] and then u = Union[u, float] will return `Union[str, int, Float.
Furthermore, I had to override getstate and setstate in UnionNode for pickle to work.

I believe there's refactoring to do in validate_and_convert() and more test cases maybe but, I think if you review this now it'll be better.

Also, _bring_type_to_front is implemented in case there's a value that can be converted in more than 1 element_type for example, 1 can be a str and an int therefore, if 1 is an int it should wrap first a IntegerNode instead of StringNode which would be incorrect.

omry

Can you check the list items?

tests/test_serialization.py

omry · 2020-11-19T18:35:37Z

tests/test_nodes.py

+            {"foo": 3.14},
+        ),
+        (
+            UnionNode(element_types=[bool, float], is_optional=True),


Optional[X] is Union[NoneType, X].
Can you make sure that that form of Union is interpreted as Optional?

I'll look into it

Done, ref_type stays as Union[NoneType, X], elements_types is without Nonetype 'Union[x]' and is_optional is True if NoneType is inside ref_type.

So actually, I just suggestd to forbid Union[NoneType, X] to keep things simple.
@omry do you see a good reason to support it?

Union[X] is exactly the same as Union[NoneType, X]:

In [1]: from typing import Union, Optional In [2]: class X: ... In [3]: Optional[X] Out[3]: typing.Union[__main__.X, NoneType] In [4]: Union[X, type(None)] == Optional[X] Out[4]: True In [5]: Union[type(None), X] == Optional[X] Out[5]: True

optional fields has special handling in OmegaConf (because their support predated Union support) but in theory it could be implemented in terms of a Union instead.

Optional[X] is exactly the same as Union[NoneType, X]:

Yes, that's why I suggested to support only Optional[X] and not Union[NoneType, X]: there is no need to support both.

Although we could probably support both, it makes things a bit more complex because now we have a Union that shouldn't translated into a UnionNode (it should instead be an XNode with optional=True).

Keeping this conversation resolved but feel free to reopen if you disagree.

Actually, unresolving this conversation because I just realized that in Python, Optional[X] is actually translated into Union[NoneType, X]. Thus I assume that OmegaConf is already catching this kind of Union somewhere to turn it into an XNode with optional=True. As a result, the error triggered at https://github.com/pereman2/omegaconf/blob/7d6e39ebed85279ef9cae6cd91bcb77748b2df22/omegaconf/_utils.py#L457 should never happen => it probably should be an assert (I don't think there was a test for it, actually).

pereman2 · 2020-11-20T16:42:27Z

I added more test cases for Union inside Dict and List

odelalleau

I'm not really familiar with the structured config code so there's some stuff I'm not too sure about.

odelalleau · 2020-11-27T04:04:04Z

omegaconf/_utils.py

+    element_types: List[Any]
+    if ref_type is not Union and args is not None and args[0] is not Any:
+        element_types = list(args)
+    assert element_types is not None


This code looks a bit weird to me:

Not sure what the if ref_type is not Union test is supposed to catch: if I remove it, all tests still pass

The assert seems to ensure that the if condition always succeeds. However, if that is not the case, then element_types will actually not be defined and you'll get an UnboundLocalError instead of an AssertionError. It would seem more natural to me to instead replace the if by an assert (on the same condition).

Not sure what the if ref_type is not Union test is supposed to catch: if I remove it, all tests still pass

It tested Unions without types(which don't make sense, like List or Dict alone now that I think about it).

I think this should be better to return Any in case it's not a union like get_element_list. Correct me if I'm wrong.

PS: Thanks for your feedback!

Not sure what the if ref_type is not Union test is supposed to catch: if I remove it, all tests still pass

It tested Unions without types(which don't make sense, like List or Dict alone now that I think about it).

I think this should be better to return Any in case it's not a union like get_element_list. Correct me if I'm wrong.

Ah ok, personally I think it'd be best to raise an exception if this function is called with anything that's not a Union[something], because I can't think of any use case where this would be intended.
Also I think that something should have at least two elements.

What is a union without a type?
like this?

@dataclass class X: a: Union

This seems like a legitimate Python (at least it runs and mypy --strict does not complain).
not sure what it's supposed to mean though. Unless someone can tell me what this is for, I think we should raise an error for a Union without a list of types.

I think we should raise an error for a Union without a list of types.

Agreed -- just tested and currently it's accepted.

omegaconf/basecontainer.py

omegaconf/nodes.py

tests/test_basic_ops_list.py

tests/test_nodes.py

tests/test_serialization.py

omegaconf/nodes.py

odelalleau

Thanks for the fixes, added a few more comments based on 720e405

…heck

omegaconf/_utils.py

Co-authored-by: Olivier Delalleau <507137+odelalleau@users.noreply.github.com>

odelalleau

Just a minor comment for now.

I've been spending some time on 45f72c0 today but it's a tricky one, touching OmegaConf's internals I'm not familiar with. Will need a bit more time to wrap my head around it.

omegaconf/nodes.py

odelalleau · 2020-11-30T04:05:14Z

omegaconf/nodes.py

+            if is_structured_config(ref_type):
+                value_type = ref_type
+            if any(
+                issubclass(value_type, union_type) for union_type in self.element_types


So there's actually an issue with this (sorry, it was my suggestion). If union_type is something like List[X] then this will crash (for instance, try executing issubclass(int, List[int]) to see the error).

I would suggest adding a test to catch this error, then either:

Write code that is robust to this (e.g., test type equality when issubclass fails)

Stick to type equality like you had earlier

A potential option:

if isinstance(value, BaseContainer): ref_type = value._metadata.ref_type if ref_type is not Any: value_type = ref_type for union_type in self.element_types: try: if issubclass(value_type, union_type): return value except TypeError: if value_type == union_type: return value self._raise_invalid_value(value, value._metadata.ref_type)

Note that I made another change, besides the subclass check: replacing if is_structured_config(ref_type) with if ref_type is not Any. To be honest I don't fully understand the role of ref_type, but intuitively I'm understanding it as "the type value is impersonating", which is the motivation behind this change (it seems more generic). Feel free to let me know if you disagree or think I'm misunderstanding how this works.

I've been adding tests and it fails as you pointed out. The code you wrote could be useful, I'll try it.
I'm finishing a sprint this week so sometime this week i expect to have a fix.

So I've been adding tests and encountered wrapping like this:

node = DictConfig({"foo": "invalid"}) _maybe_wrap(value=node._value(), ref_type=Dict[str, int], is_optional=True, key=None, parent= None)

which fails because "invalid" is AnyNode and validate_set doesn't take it well. I solved this in #443 so this might need to wait that PR.

@odelalleau, ref_type is the annotated type.
for example:

@dataclass class Foo: x : BaseClass = SubClass()

BaseClass is the ref_type.
Subclass is the type.
ref_type is checked against during assignment, for example.

I've been adding tests and it fails as you pointed out. The code you wrote could be useful, I'll try it.
I'm finishing a sprint this week so sometime this week i expect to have a fix.

I would rather that we checked if the field is a Subscripted generic and apply different logic than to catch the error and recover somehow.

So I've been adding tests and encountered wrapping like this:

node = DictConfig({"foo": "invalid"}) _maybe_wrap(value=node._value(), ref_type=Dict[str, int], is_optional=True, key=None, parent= None)

which fails because "invalid" is AnyNode and validate_set doesn't take it well. I solved this in #443 so this might need to wait that PR.

Not sure I understand, isn't it expected to fail? (since "invalid" is not an int)

It is expected to fail. and it is fixed in that other PR @pereman2 mentioned.

odelalleau · 2020-11-30T05:04:12Z

omegaconf/omegaconf.py

 ) -> Node:
    # if already a node, update key and parent and return as is.
    # NOTE: that this mutate the input node!
-    if isinstance(value, Node):
+    if isinstance(value, Node) and ref_type is Any:


So, this is the change I'm most concerned about. Until now _node_wrap() was never called (I think) with value already being a Node, and I'm a bit worried of what might happen if we allow this (especially if the code doesn't explicitly restricts this to unions).

I don't have a good solution yet, I'm still trying to figure out exactly how and when _maybe_wrap() is being used thoughout the codebase. But if there was a way to make sure that the changes required to support Union won't have unintended side effects elsewhere, I would be more confident in letting them in.

I may not have time to look more closely at it in the next few days, sorry. @omry feel free to chime in :)

Wrapping of Nodes is causing me trouble.

Wrapping of Nodes is causing me trouble.

Yeah, honestly, it seems brittle to me, because the rest of the code wasn't written with that in mind. There are a few weird things going on that I can see, and probably more that I can't.

I submitted a PR to your branch in pereman2#1 with a few things I believe make it at least a bit better. For some of my changes I struggled to write a meaningful test as I got deep into some rabbit holes, so eventually I gave up and decided that just "looking better" would be good enough ;)
(in particular, now _node_wrap() will return a UnionNode as soon as type_ is a union type, even if called from outside of UnionNode)

I doubt we'll manage to get to something 100% satisfying without a deep re-thinking of some OmegaConf internals (or completely changing the approach taken here), so as far as I'm concerned I'm ok with accepting this PR (with my suggested changes as well as the fixes you mentioned in #381 (comment)), unless @omry thinks otherwise.

IIRC, The reason maybe_wrap exists is exactly to avoid that.
wrap this object if it's not already a node. so generally I think _node_wrap should not be used on nodes (and should probably assert that).

I am going to look at the code after going through the comment and form a more informed opinion.

I think _node_wrap should not be used on nodes (and should probably assert that).

Just to give some context, currently we need it because otherwise with something like:

x: Union[int, DictConfig] = field(default_factory=lambda: DictConfig({}))

the type of node x is DictConfig instead of UnionNode, and this may break things later when we try to assign other values to x.

UnionNode is an odd beast because it's a ValueNode (not a Container) but it wraps another node -- something that didn't exist before.

* Fix assignment of a ValueNode to a UnionNode, which could sometimes results in an incorrect type * Remove the `use_type` argument to simplify the code * Keep the wrapping of a `Node` instance in `_maybe_wrap()` limited to Unions, to reduce the risk of unintended side effects

A few improvements to Unions

omry

I reviewed until the tests.
Looking pretty good so far.
Thanks @pereman2 and thanks @odelalleau for your help reviewing.

omry · 2020-12-05T18:49:05Z

omegaconf/nodes.py

+            if is_structured_config(ref_type):
+                value_type = ref_type
+            if any(
+                issubclass(value_type, union_type) for union_type in self.element_types


I've been adding tests and it fails as you pointed out. The code you wrote could be useful, I'll try it.
I'm finishing a sprint this week so sometime this week i expect to have a fix.

I would rather that we checked if the field is a Subscripted generic and apply different logic than to catch the error and recover somehow.

omry · 2020-12-05T18:49:30Z

omegaconf/nodes.py

+        return value_node
+
+    def _raise_invalid_value(self, value: Any, value_type: Any) -> None:
+        raise ValidationError(


prefer fstrings, this is Python 3 :)

done in 1b74272

You can get rid of the return None after self._raise_invalid_value(value, type(value))

Also regarding fstrings, Omry meant something like:

f"Value '{value}' with value_type '{value_type}' is not in '{self.element_types}'"

Oh shoot my bad I did it without thinking.

omry · 2020-12-05T18:53:55Z

omegaconf/nodes.py

+            )
+        )
+
+    def _get_element_types_lead_by(self, type_: Any) -> List[Any]:


this function name is unclear. what does it do?

It returns the list of union types (self.element_types) reordered so that type_ comes in first position.

omry · 2020-12-05T18:55:11Z

omegaconf/nodes.py

+        element_types[0], element_types[pos] = element_types[pos], element_types[0]
+        return element_types
+
+    def __eq__(self, other: Any) -> bool:


I think sadly python __eq__ is supposed to return Any. (because it can return NotImplemented, which is not a boolean).

How can it return NotImplemented?

How can it return NotImplemented?

Scroll down a bit to the comparison operators in https://docs.python.org/3/reference/datamodel.html#basic-customization

omry · 2020-12-05T18:58:11Z

omegaconf/nodes.py

@@ -163,6 +169,127 @@ def __deepcopy__(self, memo: Dict[int, Any] = {}) -> "AnyNode":
        return res


+class UnionNode(ValueNode):


nodes is becoming pretty big.
as a followup diff, we can refactor it and split it into a file per node (probably in omegaconf.nodes module).
e.g:

omegaconf/nodes/ any.py str.py union.py ...

Thoughts?

I agree. We should do it.

omry · 2020-12-05T18:59:40Z

omegaconf/nodes.py

+        for union_type in element_types:
+            try:
+                value_node = self._wrap_node(value=value, ref_type=union_type)
+                valid_value = True
+                break
+            except Exception:
+                pass
+        if not valid_value:
+            self._raise_invalid_value(value, type(value))
+        return value_node


A bit cleaner:

Suggested change

for union_type in element_types:

try:

value_node = self._wrap_node(value=value, ref_type=union_type)

valid_value = True

break

except Exception:

pass

if not valid_value:

self._raise_invalid_value(value, type(value))

return value_node

for union_type in element_types:

try:

return self._wrap_node(value=value, ref_type=union_type)

except Exception:

pass

self._raise_invalid_value(value, type(value))

done 1b74272.

omry · 2020-12-05T19:03:58Z

omegaconf/omegaconf.py

+    is_dict = (
+        is_dict_annotation(type_)
+        or type_ is dict
+        or (type_ is Any and type(value) is dict)


I think type_ here should actually be called ref_type.
See how it's used.

I changed it and it fails tests with ref_type

omry · 2020-12-05T19:06:03Z

omegaconf/omegaconf.py

+    # If already a node, update key and parent and return as is.
+    # NOTE: that this mutates the input node!
+    # If `ref_type` is a union we still need to wrap the node into a `UnionNode`.
+    if isinstance(value, Node) and not _is_union(ref_type):


I have a concern about this.
can this lead to multiple wrapping in a Union node?

So, right now in the test suite value is never a UnionNode here at least.

That being said, I do have concerns with unions of unions (ex: Uniont[int, Union[str, float]]). I suspect some things may not work properly. However it may not be worth supporting them since we can combine them in a single Union (maybe we should catch it and raise an error though, to be safe).

I think Python is doing it automatically. try that in an interpreter.

Oooh nice :) In that case I think it should be ok -- maybe we could add an assert not isinstance(value, UnionNode) in the else: block to be safe (unless you can see a way to trigger it in a normal use case?)

We can block this until we see an actual use case. Better having someone complain about something raising an exception than have internal structure that goes haywire in certain scenarios.

omry · 2020-12-05T19:13:31Z

tests/structured_conf/data/attr_classes.py

+@attr.s(auto_attribs=True)
+class UnionWithContainer:
+    union_dict: Union[DictConfig, int] = DictConfig({"foo": 1})
+    union_list: Union[ListConfig, int] = ListConfig([1, 2])


Add a test for a case for dataclasses as union elements sharing a common base class.
It's weird but people are doing it.

@dataclass class Base: ... @dataclass class Subclass1(Base): ... @dataclass class Subclass2(Base): ... @dataclass class UnionOfSubclasses: foo: Union[Subclass1, Subclass2]

pereman2 · 2020-12-07T17:11:47Z

Test are failing now because of I added test cases which include the bug solved in #443.

Should the try block be wrapped in a function. There are two instances of this in the same method.

odelalleau · 2020-12-07T17:18:39Z

tests/structured_conf/test_structured_config.py

+        cfg.foo = module.Subclass1
+        assert cfg.foo == module.Subclass1()


I'm a bit surprised this works, since cfg.foo is set to the class and not an instance. What's going on there?

odelalleau · 2020-12-07T17:36:39Z

omegaconf/nodes.py

+                        return self._wrap_node(
+                            value=value._value(), ref_type=union_type


I'm not sure it's a big deal but I'd rather ask @omry his opinion on this line, because it is changing the original ref_type of value to union_type (not in-place though since we create a new node, this is why it doesn't worry me too much).

omry · 2020-12-25T17:19:33Z

FYI:
I am still working on the recursive defaults in Hydra. It's a very large pull request and I am delaying reviewing of other large changes in Hydra and OmegaConf until I am done with it.
I will hopefully get it over with early after the holidays.

pereman2 · 2020-12-26T11:18:57Z

Thanks for the info. Anyways, I thought I had more time but I'm full of college work for this vacations.

omry · 2021-02-25T21:33:18Z

I bounced the Union support to OmegaConf 2.2, we will resume working on it after 2.1 is released.
Thanks for working on it.

Since many things have changed in 2.1, we will likely start from scratch but the tests will for sure be very useful.

omry reviewed Sep 21, 2020

View reviewed changes

omegaconf/_utils.py Outdated Show resolved Hide resolved

omegaconf/nodes.py Outdated Show resolved Hide resolved

omry reviewed Sep 22, 2020

View reviewed changes

omegaconf/_utils.py Outdated Show resolved Hide resolved

pereman2 force-pushed the enhance/union2 branch from a0d3cc3 to 075cb37 Compare October 15, 2020 18:01

pereman2 force-pushed the enhance/union2 branch from 075cb37 to 20dced3 Compare November 13, 2020 17:47

pereman2 added 13 commits November 17, 2020 17:00

add union support

b30eb3f

coverage

35630c6

remove comment

b12b8ff

Dicts and Lists inside Union

662772b

Change validate_value impl

4b603b3

remove prints

10495b1

__ne__

b823c2b

rebase + List and Dict with union + remove simplify validation

9037ea0

optional UnionNode

19a4012

structured types on unionnode and testmerge

025e8a8

cov

eebce7b

test fix 3.6 int bool and element_type without mutable value

1143db9

pickle union 3.6 support

b6812bc

pereman2 force-pushed the enhance/union2 branch from e05452e to b6812bc Compare November 17, 2020 16:06

pereman2 added 2 commits November 17, 2020 17:52

fix union pickle(independent nodes)

632ae0d

lint

de425ec

pereman2 requested a review from omry November 17, 2020 17:55

pereman2 changed the title ~~Add support for Union type in structured configs. [wip]~~ Add support for Union type in structured configs. Nov 19, 2020

pereman2 mentioned this pull request Nov 19, 2020

Tuple in a List (and probably Dict) does not work #436

Closed

omry reviewed Nov 19, 2020

View reviewed changes

add test cases for union in list and dict

aeefd75

omry requested a review from odelalleau November 27, 2020 00:41

odelalleau requested changes Nov 27, 2020

View reviewed changes

suggesions

720e405

odelalleau reviewed Nov 27, 2020

View reviewed changes

omegaconf/nodes.py Outdated Show resolved Hide resolved

omegaconf/nodes.py Show resolved Hide resolved

odelalleau reviewed Nov 27, 2020

View reviewed changes

pereman2 added 3 commits November 27, 2020 23:44

raise error on incorrect union annotation

1e9c3aa

elements_type as mandatory keyword arg, and add error msg

3c8edd6

performance improvement in get_element_types and remove unnecessary c…

39579cd

…heck

odelalleau reviewed Nov 28, 2020

View reviewed changes

omegaconf/_utils.py Outdated Show resolved Hide resolved

pereman2 and others added 4 commits November 28, 2020 13:21

typo and minor tweak

c09a9f1

Co-authored-by: Olivier Delalleau <507137+odelalleau@users.noreply.github.com>

fix wrong commit

6a93fcc

reconstruct Union with tuple

09f8207

containers as union type

45f72c0

odelalleau reviewed Nov 29, 2020

View reviewed changes

omegaconf/nodes.py Outdated Show resolved Hide resolved

remove unnecessary lines

0bfee7a

odelalleau reviewed Nov 30, 2020

View reviewed changes

odelalleau and others added 2 commits December 1, 2020 17:59

Merge pull request #1 from odelalleau/union_fixes

7d6e39e

A few improvements to Unions

omry reviewed Dec 5, 2020

View reviewed changes

subclass test, simplify and check subscripted

1b74272

odelalleau reviewed Dec 7, 2020

View reviewed changes

use fstrings

c5164e8

jmbuhr mentioned this pull request Dec 17, 2021

StructuredConfig from recursive dataclass #843

Closed

omry closed this Jun 22, 2023

		@@ -163,6 +169,127 @@ def __deepcopy__(self, memo: Dict[int, Any] = {}) -> "AnyNode":
		return res


		class UnionNode(ValueNode):

		cfg.foo = module.Subclass1
		assert cfg.foo == module.Subclass1()

		return self._wrap_node(
		value=value._value(), ref_type=union_type

Add support for Union type in structured configs. #381

Add support for Union type in structured configs. #381

Conversation

pereman2 commented Sep 20, 2020 • edited Loading

Tests list

omry commented Sep 20, 2020

ghost commented Nov 13, 2020 • edited by ghost Loading

👉 View analysis in DeepCode’s Dashboard | Configure the bot

pereman2 commented Nov 13, 2020

omry commented Nov 14, 2020 • edited Loading

pereman2 commented Nov 17, 2020 • edited Loading

omry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pereman2 commented Nov 20, 2020

odelalleau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odelalleau left a comment • edited Loading

Choose a reason for hiding this comment

odelalleau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

odelalleau Dec 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pereman2 Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pereman2 commented Dec 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omry commented Dec 25, 2020

pereman2 commented Dec 26, 2020

omry commented Feb 25, 2021

pereman2 commented Sep 20, 2020 •

edited

Loading

ghost commented Nov 13, 2020 •

edited by ghost

Loading

omry commented Nov 14, 2020 •

edited

Loading

pereman2 commented Nov 17, 2020 •

edited

Loading

odelalleau left a comment •

edited

Loading

odelalleau Dec 1, 2020 •

edited

Loading

pereman2 Dec 7, 2020 •

edited

Loading