Support for containers as element_type #443

pereman2 · 2020-11-26T18:16:26Z

I don't know if this should close #436 and #427 since both mention Tuple which still isn't implemented as a container.

Maybe I should move merge tests to test_merge?

With respect to the should_assign, I think it's necessary because if input_config it's true and element_type is a container it assigns instead of wrapping.

omegaconf/basecontainer.py

omry · 2020-11-26T18:39:47Z

omegaconf/dictconfig.py

+        is_valid_target = not target_has_ref_type and not (
+            isinstance(value, BaseContainer)
+            and is_structured_config(value._metadata.ref_type)
+            and self._metadata.element_type is not Any
+        )


what case is this change needed for? that's a whole lot of new conditions and I don't understand why we need them.

omry · 2020-11-26T18:45:14Z

omegaconf/listconfig.py

+        if is_structured_config(target_type) or (
+            target_type is not Any and isinstance(value, ValueNode)
+        ):
            if (
                target_type is not None
                and value_type is not None
                and not issubclass(value_type, target_type)
            ):
-                msg = (
-                    f"Invalid type assigned : {type_str(value_type)} is not a "
-                    f"subclass of {type_str(target_type)}. value: {value}"
-                )
-                raise ValidationError(msg)
+                raise_validation_error(value_type, target_type, value)
+        if (
+            is_structured_config(value_type)
+            and not is_structured_config(target_type)
+            and target_type not in (Any, None)
+        ):
+            raise_validation_error(value_type, target_type, value)


this block is confused. can you reorganized it to be clearer? (adding a comment about which failure case each condition is detecting will also help).

tests/structured_conf/test_structured_config.py

omegaconf/listconfig.py

tests/structured_conf/test_structured_config.py

omry · 2020-12-01T18:21:01Z

omegaconf/basecontainer.py

+            if input_config:
+                val = val._value()
            return _maybe_wrap(
                ref_type=ref_type,
                key=key,


If input is a config why do you unwrap and wrap it?

That's because maybe wrap doesn't really wrap nodes and we need to check if the container value(input_config) has the correct types given a ref_type.

If we remove this the following test case fails(merging complex dicts):

( "ComplexDict", # Dict[str, Dict[str, List[int]]] { "dict": {"foo2": {"var2": ["invalid", 4]}}, }, ), # merge with ComplexDict

omry · 2020-12-01T18:25:42Z

omegaconf/dictconfig.py

+        is_valid_target = not target_has_ref_type and not (
+            (
+                is_structured_config(value_type)
+                and self._metadata.element_type is not Any
+            )
+            or (isinstance(value, AnyNode) and target_type is not Any)
+        )


too much. add some temporary boolean variables to make this clearer.

omry · 2020-12-01T18:27:38Z

omegaconf/listconfig.py

+        if (
+            is_structured_config(value_type)
+            or is_structured_config(target_type)
+            or isinstance(value, ValueNode)
+        ):
+            if value_type is not None and not issubclass(value_type, target_type):
+                raise_validation_error(value_type, target_type, value)


This is bad form:

if A: if B: ...

you can do:

if A and B: ...

I thought in this case it would be clearer this way.

omry · 2020-12-01T18:29:45Z

omegaconf/listconfig.py

+            element_type = self.__dict__["_metadata"].element_type
+            value = item
+            expect_container = is_container_annotation(element_type)
+            if expect_container and isinstance(item, BaseContainer):
+                value = item._value()
+
            node = _maybe_wrap(
                ref_type=self.__dict__["_metadata"].element_type,
                key=index,
-                value=item,
+                value=value,
                is_optional=OmegaConf.is_optional(item),
                parent=self,
            )


What is this doing?
It looks like if you get a DictConfig or ListConfig you use the content directly and wrap it again.
What is this achieving?

As I said above(and other PRs), if you merge two containers you have to deal with maybe_wrap which won't wrap nodes.

We want to type check whether the contents of the value if we expect the value to be a container, with certain element_type and key_type. The solution I found to work here and other parts of the code(like input_config above) is to unwrap to check again.

A possibly cleaner solution is to treat the case of a DictConfig assigned onto a DictConfig by validating that the key type and the element types of the src are compatible with those in the target.

Maybe we can implement a function like:

is_legal_assignment(dest_type, src_type) -> bool

To handle that.
That function can be used to check if the type of a value can be assigned to a specific target type, but can also be used to check if Dict[str, int] can be assigned to Dict[str, Any] (yes) or to Dict[int, int] (no).

There is no need to re-wrap the DictConfig. we can know that all of keys and values are conforming to the specified key type and element type.

Do you mean this function should check the type of every value in DictConfig?

I mean that a DictConfig can be assumed to be legal.
There was validation performed on it's input when it was created.
When you are assigning a DictConfig, you should not need to recursively re-validate it.

You can look at the key type and element types:
if they are matching (or more specific), the assignment is legal.

omry · 2020-12-05T20:56:56Z

omegaconf/dictconfig.py

+        input_container = (
+            is_structured_config(value_type) and self._metadata.element_type is not Any
+        )


this does not read as an input container to me.
What case are you detecting here?

Oops, it should be input_structured_config, what do you think?
If value_type is a structured and we have an element_type it should do issubclass(value_type, target_type) which didn't happen because this case was considered a "valid_value".

self._metadata.element_type can also be a primitive type. (int, str etc).
I think you should reorganize the logic here a bit.

omry · 2020-12-05T21:02:24Z

omegaconf/listconfig.py

+        def raise_validation_error(
+            value_type: Any, target_type: Any, value: Any
+        ) -> None:
+            msg = (
+                f"Invalid type assigned : {type_str(value_type)} is not a "
+                f"subclass of {type_str(target_type)}. value: {value}"
+            )
+
+            raise ValidationError(msg)


Adding this function here, which is only called once - is making it even harder to understand what you are actually changing here.
If it's only called once, what is the point of extracting it into a local function?

Looking back at it doesn't make sense. I think at one point I reused this function(while testing) but now not. I'll remove it.

Sounds good.

omry · 2020-12-05T21:03:45Z

omegaconf/listconfig.py

+        if (
+            (
+                is_structured_config(value_type)
+                or is_structured_config(target_type)
+                or isinstance(value, ValueNode)
+            )
+            and value_type is not None
+            and not issubclass(value_type, target_type)
+        ):
+            raise_validation_error(value_type, target_type, value)


Can you explain what new cases this is dealing with?
it's significantly more complicated than before and looking at it I suspect it's actually too broad.

Well if we have a structured config the way to check is with issubclass so that's one case.

The case with ValueNodes is because the same reason with other PRs where we receive a node instead of a primitive value and, this is where we can check.

Trying to parse the logic here:

if ( ( is_structured_config(value_type) or is_structured_config(target_type) or isinstance(value, ValueNode) )

If: input is structured config (e.g an instance of or a dataclass) or element type is a structured config or value we are assigning a value node

This is already lumping multiple unrealted things together.

and value_type is not None and not issubclass(value_type, target_type)

and the value type is not a subclass of the element type.
=> fail.

To me a better way to express this sounds like:

if value is a structured config: if element type is a structured config and value type is not a subclass => fail

I don't know what the input being a value node has to do with it, shouldn't the check be performed on the unboxed input?

Trying to parse the logic here:

if ( ( is_structured_config(value_type) or is_structured_config(target_type) or isinstance(value, ValueNode) )

If: input is structured config (e.g an instance of or a dataclass) or element type is a structured config or value we are assigning a value node

This is already lumping multiple unrealted things together.

and value_type is not None and not issubclass(value_type, target_type)

and the value type is not a subclass of the element type.
=> fail.

To me a better way to express this sounds like:

if value is a structured config: if element type is a structured config and value type is not a subclass => fail

I agree.

I don't know what the input being a value node has to do with it, shouldn't the check be performed on the unboxed input?

ValueNodes are a bit tricky because of implicit conversion. I think this process could be much more simplified with :

if value is not a subscripted container type and not subclass..

But since value nodes implicitly convert values we cannot. Therefore, we arrive to a point where in the following case

@attr.s(auto_attribs=True) class ComplexDict: dict: Dict[str, Dict[str, List[int]]] = {"foo": {"var": [1, 2]}} obj = { "dict": {"foo2": {"var2": ["invalid", 4]}}, } c = OmegaConf.structured(ComplexDict) OmegaConf.merge(c, obj) # no error

This doesn't raise an error because "invalid" is an StringNode which cannot be checked its value without defining an explicit check in _validate_set and, if we check with the unboxed value(str literal) it can cause regression problems since, for example, the int value 4 can be StringNode too(issubclass(int, str) will fail but this is ok in omegaconf).

I don't know if I explained this correctly, I hope this one clarifies my concerns.

omry · 2020-12-05T21:11:45Z

omegaconf/listconfig.py

+            element_type = self.__dict__["_metadata"].element_type
+            value = item
+            expect_container = is_container_annotation(element_type)
+            if expect_container and isinstance(item, BaseContainer):
+                value = item._value()
+
            node = _maybe_wrap(
                ref_type=self.__dict__["_metadata"].element_type,
                key=index,
-                value=item,
+                value=value,
                is_optional=OmegaConf.is_optional(item),
                parent=self,
            )


A possibly cleaner solution is to treat the case of a DictConfig assigned onto a DictConfig by validating that the key type and the element types of the src are compatible with those in the target.

Maybe we can implement a function like:

is_legal_assignment(dest_type, src_type) -> bool

To handle that.
That function can be used to check if the type of a value can be assigned to a specific target type, but can also be used to check if Dict[str, int] can be assigned to Dict[str, Any] (yes) or to Dict[int, int] (no).

There is no need to re-wrap the DictConfig. we can know that all of keys and values are conforming to the specified key type and element type.

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

omegaconf/listconfig.py

@@ -234,10 +248,16 @@ def append(self, item: Any) -> None:
            index = len(self)
            self._validate_set(key=index, value=item)

+            element_type = self.__dict__["_metadata"].element_type
+            value = item


What is this assignment achieves?

Please clear this assignment if it does not do anything useful.

omry · 2020-12-25T17:25:32Z

tests/structured_conf/test_structured_config.py

+                "ComplexList",
+                {
+                    "list": [[{"foo": "invalid"}]],
+                },
+            ),
+            (
+                "ComplexList",
+                {
+                    "list": [[{"foo": User()}]],
+                },
+            ),
+            (
+                "ComplexDict",
+                {
+                    "dict": {"foo2": {"var2": ["invalid", 4]}},
+                },
+            ),
+        ],


try to remove trialing comma from lines like:

"list": [[{"foo": "invalid"}]],

I will likely result in a more compact formatting.

omry · 2020-12-25T17:29:30Z

omegaconf/listconfig.py

+            element_type = self.__dict__["_metadata"].element_type
+            expect_container = is_container_annotation(element_type)
+            if expect_container and isinstance(item, BaseContainer):
+                item = item._value()
+


I am not seeing new tests for deepcopy/copy that are testing with containers as element types.
Can you add some?

omry · 2020-12-25T17:31:50Z

omegaconf/listconfig.py

        target_type = self._metadata.element_type
+        if target_type in (Any, None):
+            return
+
        value_type = OmegaConf.get_type(value)
-        if is_structured_config(target_type):
-            if (
-                target_type is not None
-                and value_type is not None
-                and not issubclass(value_type, target_type)
-            ):
-                msg = (
-                    f"Invalid type assigned : {type_str(value_type)} is not a "
-                    f"subclass of {type_str(target_type)}. value: {value}"
-                )
-                raise ValidationError(msg)
+
+        if (
+            (
+                is_structured_config(value_type)
+                or is_structured_config(target_type)
+                or isinstance(value, ValueNode)
+            )
+            and value_type is not None
+            and not issubclass(value_type, target_type)
+        ):
+            msg = (
+                f"Invalid type assigned : {type_str(value_type)} is not a "
+                f"subclass of {type_str(target_type)}. value: {value}"
+            )
+
+            raise ValidationError(msg)



This still looks too complicated.
can you try to make it more readable by refacotring it a bit? (maybe extract local boolean values and use them in conditions like in DictConfig).

pereman2 · 2021-01-26T18:02:14Z

omegaconf/listconfig.py

+        needs_type_validation = is_structured_config(value_type) or isinstance(
+            value, ValueNode
+        )
+        if (
+            needs_type_validation
+            and value_type is not None
+            and not issubclass(value_type, target_type)
+        ):
+            msg = (
+                f"Invalid type assigned : {type_str(value_type)} is not a "
+                f"subclass of {type_str(target_type)}. value: {value}"
+            )


I extracted this variable. Do you think I should leave a comment explaining why we need needs_type_validation ?

pereman2 · 2021-01-27T07:25:18Z

omegaconf/listconfig.py

+            element_type = self.__dict__["_metadata"].element_type
+            if is_container_annotation(element_type) and isinstance(
+                item, BaseContainer
+            ):
+                item_ref_type = item._metadata.ref_type
+                if is_container_annotation(item_ref_type) and not is_legal_assignment(
+                    element_type, item_ref_type
+                ):
+                    _raise_invalid_assignment(element_type, item_ref_type, item)
+                else:
+                    # regular dict without annotation
+                    item = item._value()
+


I added the function is_legal_assignment in utils.py to not re-wrap as you mentioned. Even though we check with is_legal_assignment, it's necessary a default case where we re-wrap in case it's a regular dict. For example:

The following snippet where dis a DictConfig without a Dict annotation, if there wasn't a default option where it re-wraps the DictConfig, it wouldn't be possible to find if it's a legal assignment.

@dataclass class ContainerInList: list_with_list: List[List[int]] = field(default_factory=lambda: [[0, 1], [2, 3]]) list_with_dict: List[Dict[str, int]] = field( default_factory=lambda: [{"foo": 1, "foo2": 2}] ) d = {"list_with_list": [[4, 5]], "list_with_dict": [{"foo": User()}]} cfg = OmegaConf.structured(ContainerInList) OmegaConf.merge(cfg, d) # no error

Re-wrapping wouldn't be necessary if somehow we built ref_type, element_type, key_type on runtime for DictConfigs like d. I don't know if this is impossible to do.

Moreover, I added a test case example in 0b9cbfe which is the only one that covers this example. I'll wait for your review and then, complete with more test cases like the one in 0b9cbfe.

omry · 2021-01-27T20:05:08Z

omegaconf/dictconfig.py

+        input_container = (
+            is_structured_config(value_type) and self._metadata.element_type is not Any
+        )


self._metadata.element_type can also be a primitive type. (int, str etc).
I think you should reorganize the logic here a bit.

omry · 2021-01-27T20:08:11Z

omegaconf/listconfig.py

-                )
-                raise ValidationError(msg)
+
+        needs_type_validation = is_structured_config(value_type) or isinstance(


what if the input is a primitive, say int. does it mean that it does not need type validation?

omry · 2021-01-27T20:22:58Z

omegaconf/basecontainer.py

@@ -747,6 +751,23 @@ def prepand(full_key: str, parent_type: Any, cur_type: Any, key: Any) -> str:

        return full_key

+    # If item has a Container annotation like Dict[str, int] it validates against the target element_type
+    # if it doesn't have an annotation it returns the raw value
+    def _resolve_container_assignment(self, item: Any) -> Any:


This function is doing too much, which also leads to it having an opaque name.
It's doing some assignment validation (and raising an exception), and unwrappingg the object in some cases.

Assignment validation is done in validate_set. It's not clear to me why you want to perform a partial validation from various places.
Unwrapping can happen as a part of assignment. (_set_value() on DictConfig and ListConfig).

I'm going to tackle this between today and tomorrow.

omry · 2021-01-27T20:24:48Z

omegaconf/dictconfig.py

@@ -199,7 +211,7 @@ def _validate_set(self, key: Any, value: Any) -> None:
            and not issubclass(value_type, target_type)
        )
        if validation_error:
-            self._raise_invalid_value(value, value_type, target_type)
+            _raise_invalid_assignment(target_type, value_type, value)


Is this a fix to facebookresearch/hydra#1322 by any chance?

No, in dictconfig.py:629

else: # pragma: no cover msg = f"Unsupported value type : {value}" raise ValidationError(msg)

should print type_str(type(value)). Should I fix it in this PR?

I am not sure it's that simple. we can do it in a different pr (need a clean standalone repro first).

omry · 2021-01-27T20:26:46Z

omegaconf/listconfig.py

            self._validate_set(key=index, value=item)

+            item = self._resolve_container_assignment(item)
+
            node = _maybe_wrap(
                ref_type=self.__dict__["_metadata"].element_type,
                key=index,


see, here for example the responsibility of _resolve_container_assignment() can be split between validate_set() above and _maybe_wrap() below.

omry · 2021-01-27T20:37:42Z

tests/structured_conf/test_structured_config.py

+        ],
+    )
+    def test_container_as_element_type_valid(
+        self, class_type: str, obj: str, value: Any, node: str


rename node to key

omry · 2021-01-27T20:38:28Z

tests/structured_conf/test_structured_config.py

+    def test_container_as_element_type_invalid(
+        self, class_type: str, obj: str, value: Any, node: str
+    ) -> None:


rename node to key

omry · 2021-01-27T20:40:06Z

tests/structured_conf/test_structured_config.py

+
+    @pytest.mark.parametrize(


This is a ton of new tests, good job.
Move them to a dedicated file though.

test_structured_config_container_element.py?

omry · 2021-01-27T20:41:04Z

tests/test_basic_ops_dict.py

+        pytest.param("dict_with_list", [3, 4], id="copy_dict_with_list"),
+    ],
+)
+def test_copy_container_in_dict(node: str, value: Any) -> None:


node -> key

omry · 2021-01-27T20:41:44Z

tests/test_basic_ops_list.py

+
+
+@pytest.mark.parametrize(
+    "node, value",


node -> key

omry · 2021-01-29T19:08:20Z

tests/structured_conf/data/attr_classes.py

+class ContainerInDictWithUser:
+    dict_with_list: Dict[str, List[User2]] = {"foo": [User2()]}
+    dict_with_dict: Dict[str, Dict[str, User2]] = {"foo": {"var": User2()}}
+
+
+@attr.s(auto_attribs=True)
+class ContainerInListWithUser:


class names here seems wrong.
Is this a container IN dict with user or the other way around?

Does ContainerWithUserInDict look good for you ?

DictWithNestedUserContainer?

omry · 2021-02-02T18:54:26Z

omegaconf/dictconfig.py

@@ -199,7 +211,7 @@ def _validate_set(self, key: Any, value: Any) -> None:
            and not issubclass(value_type, target_type)
        )
        if validation_error:
-            self._raise_invalid_value(value, value_type, target_type)
+            _raise_invalid_assignment(target_type, value_type, value)


I am not sure it's that simple. we can do it in a different pr (need a clean standalone repro first).

pereman2 · 2021-02-03T11:59:00Z

I've added more test cases in fa2c0a2 which are failing. This was after reviewing coverage output.

omry

I have an overall feeling that this is more complicated than it should be, and that things gets more complicated as I am providing more feedback.

Maybe this is too much for a single PR and we should break it down vertically (support Dict, List and maybe Tuple), each with a standalone PR.
what do you think?

omry · 2021-02-10T22:29:50Z

omegaconf/_utils.py

@@ -748,6 +748,47 @@ def is_container_annotation(type_: Any) -> bool:
    return is_list_annotation(type_) or is_dict_annotation(type_)


+def is_container_assignment(cfg: Any) -> bool:


This function name makes zero sense outside of the context it's used.

a cfg is not a container assignment.

omry · 2021-02-10T22:31:53Z

omegaconf/_utils.py

+    if is_container_annotation(element_type):
+        return True
+    else:
+        return False


why not

element_type = cfg.__dict__["_metadata"].element_type return is_container_annotation(element_type):

or even

return is_container_annotation(cfg.__dict__["_metadata"].element_type)

Also, why are you accessing the metadata through dict and not through cfg._metadata?

omry · 2021-02-10T22:33:19Z

omegaconf/_utils.py

+    return False
+
+
+def should_unwrap(cfg: Any, value: Any) -> bool:


I am still not clear why we are wrapping in the first place if we need to unwrap.
can we just not wrap in this scenario? we will be able to eliminate a lot of new logic.

omry · 2021-02-10T22:37:01Z

omegaconf/listconfig.py

+        ) or (
+            is_container_assignment(self)
+            and is_invalid_container_assignment(self, value)


You are only using is_invalid_container_assignment once, in ListConfig. is it not needed in DictConfig?

omry · 2021-02-10T22:44:55Z

omegaconf/_utils.py

+    else:
+        return False
+
+
 def is_legal_assignment(dest_type: Any, src_type: Any) -> bool:


is_legal_assignment in _utils sounds like a very generic function checking if assignment of a source type is legal to a dest type.
It only supports container types though.
It's also implemented in a verbose way.

The following code is not tested. just making a point:

# test direct assignment. def is_legal_assignment(dest_type: Any, src_type: Any) -> bool: return issubclass(src_type, dest_type) or dest_type is Any # assignment of a value to a typed list: valid = is_legal_assignment(lst._metadata.element_type, value_type) # assignment of a value to a typed dict valid = is_legal_assignment(dct._metadata.element_type, value_type)

Your implementation for this includes many functions that are written in a very verbose way.
Seems like it can be much simpler.

omry · 2021-02-10T22:58:48Z

omegaconf/_utils.py

+    is_legal = False
+    if is_list_annotation(dest_type) and is_list_annotation(src_type):
+        is_legal = is_legal_list_assignment(dest_type, src_type)
+    elif is_dict_annotation(dest_type) and is_dict_annotation(src_type):
+        is_legal = is_legal_dict_assignment(dest_type, src_type)
+    return is_legal


Point on style: This is cleaner in this scenario:

if is_list_annotation(dest_type) and is_list_annotation(src_type): return is_legal_list_assignment(dest_type, src_type) elif is_dict_annotation(dest_type) and is_dict_annotation(src_type): return is_legal_dict_assignment(dest_type, src_type) return False

omry · 2021-02-10T23:01:24Z

omegaconf/_utils.py

+def is_legal_dict_assignment(dest_type: Any, src_type: Any) -> bool:
+    key_values_pair_dest = get_dict_key_value_types(dest_type)
+    key_values_pair_src = get_dict_key_value_types(src_type)
+    return key_values_pair_dest == key_values_pair_src


Isn't this just dest_type == src_type?

Jasha10 · 2022-04-13T03:24:53Z

Closing this as #890 implemented support for containers as element_type.

omry requested changes Nov 26, 2020

View reviewed changes

omry reviewed Dec 1, 2020

View reviewed changes

omegaconf/listconfig.py Outdated Show resolved Hide resolved

tests/structured_conf/test_structured_config.py Outdated Show resolved Hide resolved

omry requested changes Dec 1, 2020

View reviewed changes

pereman2 mentioned this pull request Dec 3, 2020

Add support for Union type in structured configs. #381

Closed

6 tasks

omry reviewed Dec 5, 2020

View reviewed changes

pereman2 force-pushed the 436 branch from 1c50ab0 to 09a59fa Compare December 20, 2020 16:44

omry requested changes Dec 25, 2020

View reviewed changes

pereman2 force-pushed the 436 branch from 9cc92e1 to 17f105a Compare January 26, 2021 16:10

pereman2 commented Jan 26, 2021

View reviewed changes

pereman2 commented Jan 27, 2021

View reviewed changes

omry requested changes Jan 27, 2021

View reviewed changes

pereman2 added 15 commits February 2, 2021 12:41

tests and container as element_type

4b293ef

complex container nesting

e798713

small change

4ac1618

simplify a bit

ce1c448

boolean variables(valid_target) and join ifs

2ef94ca

remove raise function

36d455a

remove unnecessary assignment

8770d10

lint

3b8ede1

remove trailing commas in test cases

70e07b7

test copy

1ec53e0

extract variable needs_type_validation

e86d3e2

is_valid_assignment check to containers

e16d4a6

add test case

481087b

refactor a bit and add test case

31f42a8

move test to new file and add SC test cases

a92792c

pereman2 force-pushed the 436 branch from 1528633 to a92792c Compare February 2, 2021 11:45

pereman2 added 2 commits February 2, 2021 12:53

quick rebase fix

4953edc

remove unused function and remove None check

ab7e495

omry reviewed Feb 2, 2021

View reviewed changes

pereman2 added 2 commits February 3, 2021 10:59

divide resolve responsibilities in validate_set and append

0de6dbb

more test cases which fail, wip fix

fa2c0a2

omry reviewed Feb 10, 2021

View reviewed changes

Jasha10 closed this Apr 13, 2022

		@@ -748,6 +748,47 @@ def is_container_annotation(type_: Any) -> bool:
		return is_list_annotation(type_) or is_dict_annotation(type_)


		def is_container_assignment(cfg: Any) -> bool:

		return False


		def should_unwrap(cfg: Any, value: Any) -> bool:

Support for containers as element_type #443

Support for containers as element_type #443

Conversation

pereman2 commented Nov 26, 2020

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 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

pereman2 Jan 26, 2021 • 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

pereman2 commented Feb 3, 2021

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

Jasha10 commented Apr 13, 2022

pereman2 Dec 7, 2020 •

edited

Loading

pereman2 Jan 26, 2021 •

edited

Loading