Support type aliases in fine-grained incremental mode #4525

ilevkivskyi · 2018-01-31T20:12:31Z

This was a bit more tricky than I expected. I think I cover all common cases and majority of corner cases:

Simple aliases
Generic aliases
Forward references
Nested aliases
Chained aliases

Note that among tests I added some also pass without this addition (probably because some dependencies are added by coincidence). Still there are 17 test scenarios that would fail without this change. Note that I mainly focus on false negatives, since in my experience while playing with fine-grained daemon, this is the most typical problem.

There are two weird failures that I skip, but those seem to be independent of type aliases (I checked manually that alias dependence maps are correct). The problem goes away if I just "touch" the aa.py module.

… tests

…ses; add tests" This reverts commit 8d42ca7.

JukkaL

Looks mostly good, just a bunch of minor things. Also, should there be some AST diff test cases for type aliases as well? What about AST merge test cases?

JukkaL · 2018-01-31T21:16:06Z

mypy/nodes.py

+    # Is this node refers to other node via node aliasing?
+    # (This is currently used for simple aliases like `A = int` instead of .type_override)
+    is_aliasing = False  # type: bool
+    alias_depends_on = None  # type: Set[str]


Add docstring for alias_depends_on.

There are starting to be quite a few alias-related attributes in symbol table nodes. Maybe it's time to add a new AST node for type aliases soon after this PR has been merged so that we can remove several of these attributes?

Maybe it's time to add a new AST node for type aliases soon after this PR has been merged so that we can remove several of these attributes?

Yes, I will take care of it as part of #4082

JukkaL · 2018-01-31T21:25:58Z

mypy/semanal.py

+    def add_alias_deps(self, aliases_used: Set[str]) -> None:
+        """Add full names of type aliases on which the current node depends.
+
+        This is used by fine-grained incremental mode to re-chek the corresponding nodes.


Typo: re-chek

JukkaL · 2018-01-31T21:29:51Z

mypy/semanal.py

@@ -1178,7 +1182,8 @@ def update_metaclass(self, defn: ClassDef) -> None:
            return
        defn.metaclass = metas.pop()

-    def expr_to_analyzed_type(self, expr: Expression) -> Type:
+    def expr_to_analyzed_type(self, expr: Expression, *,
+                              from_bases: Optional[ClassDef] = None) -> Type:


The name of argument from_bases is unclear. Maybe rename it and also document in a docstring.

JukkaL · 2018-01-31T21:30:18Z

mypy/semanal.py

@@ -1674,12 +1679,32 @@ def anal_type(self, t: Type, *,
                  tvar_scope: Optional[TypeVarScope] = None,
                  allow_tuple_literal: bool = False,
                  aliasing: bool = False,
-                  third_pass: bool = False) -> Type:
+                  third_pass: bool = False,
+                  from_bases: Optional[ClassDef] = None) -> Type:


Another instance of the from_bases that could be renamed.

JukkaL · 2018-01-31T21:32:21Z

mypy/semanal.py

+                self.cur_mod_node.alias_deps[from_bases].update(a.aliases_used)
+                return tp
+            self.add_alias_deps(a.aliases_used)
+        return tp


The logic is a bit convoluted. What about rewriting this like this:

if a.aliases_used: if from_bases is not None: self.cur_mod_node.alias_deps[from_bases].update(a.aliases_used) else: self.add_alias_deps(a.aliases_used) return t.accept(a)

JukkaL · 2018-01-31T23:36:40Z

test-data/unit/deps-types.test

+
+[case testAliasDepsGenericMod]
+from typing import Dict
+A = Dict[int, str]


Would be good to have a user-defined generic type such as C[D, E] to get more dependencies.

JukkaL · 2018-01-31T23:37:32Z

test-data/unit/deps-types.test

+    x: A[int]
+[builtins fixtures/dict.pyi]
+[out]
+<m.A> -> m


Dependency from T to A is missing. For example, if T gets removed A should become stale (T could be defined in another target).

JukkaL · 2018-01-31T23:37:58Z

test-data/unit/deps-types.test

+
+[case testAliasDepsForwardMod]
+x: A
+A = int


Again, would be good to have an alias to a user-defined type for more generated dependencies.

JukkaL · 2018-01-31T23:58:45Z

test-data/unit/deps-types.test

+[out]
+<m.A> -> m, m.f
+<m.B> -> m, m.f
+


Some ideas for additional test cases (not sure if all of these are worth adding):

Alias used as a cast target

Alias used in x = Foo[Alias]()

Alias used in NamedTuple definition

Alias used in TypeVar (bound or values)

Alias used in TypedDict definition

TypeVars are currently supported only partially (independently of aliases), and TypedDict has only basic support. I will therefore add only few basic tests with them. For others yes, I will add more tests.

JukkaL · 2018-02-01T00:00:49Z

test-data/unit/fine-grained.test

+[out]
+==
+b.py:2: error: Dict entry 0 has incompatible type "str": "int"; expected "str": "str"
+


Ideas for additional test cases:

Alias changed into a generic alias (and vice versa)

Changing the number of type args of a generic alias (e.g. C[T] -> C[T, S])

Deleting the target of a type alias or a component of the type alias target

ilevkivskyi · 2018-02-01T13:58:37Z

Also, should there be some AST diff test cases for type aliases as well? What about AST merge test cases?

I thought that these are less important, since aliases should be completely expanded after semantic analysis, I think I will add few tests for these.

ilevkivskyi · 2018-02-01T13:59:05Z

@JukkaL Thanks for a detailed review! I will try to finish this today.

…eVar fullname. Also maybe record exact location of type alias in semanal to fire attribute triggers.

ilevkivskyi · 2018-02-08T20:29:12Z

@JukkaL I added the tests you requested and fixed corner cases. Please review again.

I am still not fluent in how dependencies are (re-)triggered, so I am not sure I choose the right balance between completeness and performance (i.e. many vs few dependencies). Please advise on this.

ilevkivskyi · 2018-02-08T20:40:07Z

Hm, strange, all tests passed locally, but here in both Travis and AppVeyor errors are sorted in different order in one test. Should we somehow fix the order they are shown? (To make tests more stable.)

This reverts commit b3a0488.

ilevkivskyi · 2018-02-08T21:07:17Z

Sorry, I give up, this is beyond my understanding, all tests fail consistently. Independently of what is the order, the messages are shown in the opposite order.

JukkaL

Thanks for the updates! This is now much cleaner.

Can you add AST diff test cases as well (test-data/unit/diff.test)?

JukkaL · 2018-02-19T13:35:55Z

mypy/scope.py

@@ -0,0 +1,80 @@
+"""Track current scope to easily calculate the corresponding fine-grained target.
+
+This is currently only used by mypy.semanal and mypy.server.deps.


This comment can easily become stale. What about rewording as "TODO: Use everywhere where we track targets, including in mypy.errors".

JukkaL · 2018-02-19T13:38:07Z

mypy/semanal.py

        """Check if 'rvalue' represents a valid type allowed for aliasing
-        (e.g. not a type variable). If yes, return the corresponding type and a list of
-        qualified type variable names for generic aliases.
+        (e.g. not a type variable). If yes, return the corresponding type, a list of


Style nit: Use an empty line after the first line in a multi-line docstring.

JukkaL · 2018-02-19T13:58:30Z

test-data/unit/deps-types.test

+[case testAliasDepsNormalMod]
+from mod import I
+A = I
+x: A


To get a fuller picture of aliases, you could move the alias definition to a separate module (so that there would be 3 modules: class definition, alias definition, and alias use in annotation).

JukkaL · 2018-02-19T13:59:42Z

test-data/unit/deps-types.test

+<mod.I> -> <m.A>, <m.x>, m
+
+[case testAliasDepsNormalFunc]
+from mod import I


Similar to above, it would be helpful to have 3 modules here. This is probably worth it for some other test cases below as well (up to you).

JukkaL · 2018-02-19T14:05:46Z

mypy/nodes.py

+    # (This is currently used for simple aliases like `A = int` instead of .type_override)
+    is_aliasing = False  # type: bool
+    # This includes full names of aliases used in this alias.
+    alias_depends_on = None  # type: Set[str]


I wonder if this actually is necessary. Even without this, the target that contains the alias definition should depend on these aliases, right? And change in the alias should be detected by mypy.server.astdiff. If you remove this, what will break? Maybe some symbol table differences won't then be propagated correctly -- but that could just be a bug elsewhere, which this papers over.

Ideally this shouldn't be necessary. But IIRC correctly some tests failed without this. I added a TODO about this in typeanal.py (that you also commented). I will try to remove this and re-run the tests, and if some will still fail, I will try to figure out where exactly is the problem in diff calculation and expand the TODO.

JukkaL · 2018-02-19T14:22:55Z

mypy/server/deps.py

@@ -506,6 +537,13 @@ def visit_yield_from_expr(self, e: YieldFromExpr) -> None:

    # Helpers

+    def add_type_alias_deps(self, target: str, add_trigger: bool = False) -> None:


The add_trigger argument name is unclear. Can you come up with another name? Maybe something about triggering recursively?

JukkaL · 2018-02-19T14:24:10Z

mypy/server/deps.py

                self.add_dependency(trigger)
                self.add_dependency(trigger, target=make_trigger(target))
        if o.info:
            for base in non_trivial_bases(o.info):
                self.add_dependency(make_trigger(base.fullname() + '.' + o.name()))
+        self.add_type_alias_deps(self.scope.current_full_target(), add_trigger=True)


Add comment about why we have add_trigger=True. Again, I believe that this should be current_target() instead of current_full_target().

JukkaL · 2018-02-19T14:25:46Z

mypy/server/deps.py

    #   functional enum
    #   type variable with value restriction

    def visit_mypy_file(self, o: MypyFile) -> None:
        self.scope.enter_file(o.fullname())
        self.is_package_init_file = o.is_package_init_file()
+        self.add_type_alias_deps(self.scope.current_full_target())


This should be self.scope.current_target()? current_full_target() can refer to an entire class (all method bodies) which we want to avoid if possible, since it's expensive.

JukkaL · 2018-02-19T14:26:34Z

mypy/server/deps.py

@@ -202,6 +221,7 @@ def visit_class_def(self, o: ClassDef) -> None:
            self.add_type_dependencies(o.info.typeddict_type, target=make_trigger(target))
        # TODO: Add dependencies based on remaining TypeInfo attributes.
        super().visit_class_def(o)
+        self.add_type_alias_deps(self.scope.current_full_target())


Similar to above, this should probably be current_target().

JukkaL · 2018-02-19T14:27:43Z

mypy/typeanal.py

+        # Names of type aliases encountered while analysing a type will be collected here.
+        # (This also recursively includes the names of aliases they depend on.)
+        # TODO: adding the sub-dependencies shouldn't be necessary since the logic
+        # of fine-grained should propagate triggered dependencies.


Why is this necessary? It would be useful to understand the precise root cause for why this is included and mention it in the TODO comment.

ilevkivskyi · 2018-02-19T23:28:04Z

@JukkaL OK, I got rid of the "long-range" (i.e. indirect, recursive) dependencies for type aliases. There are few minor remaining questions we can discuss tomorrow (also I will add tests you asked). Here is a summary of how everything works:

Definition of a type alias creates a dependency for the enclosing scope, so that the alias will be reprocessed if it is changed. So A = int generates <A> -> __main__.
Use of a type alias creates a dependency for the using target on the alias. So def func(x: A): ... generates <A> -> func (for functions this also creates <A> -> <func>, but this is not something special for aliases, the same happens for other types because functions are not correctly treated in get_triggered_namespace_items)
Finally, type aliases depend on their expansion, aliases directly used, and type variables (for generic aliases). So, B = Dict[A, Cls] generates <A> -> , <Cls> ->

This is all, no other dependencies are generated, for "chained" aliases etc. indirect dependencies are re-triggered and re-processed in several passes.

There is one thing that concerns me: currently subscripted and un-subscripted aliases are processed differently so this also required some duplication of logic (this also happens in several other places). As we discussed, there will be refactoring of symbol table nodes, so there are two options:

First refactor symbol table nodes, then rebase this on top which will avoid duplication of logic.
First merge this, then remove duplication of logic here as part of the refactoring.

I prefer the second option, since all this picture of dependencies is now in my head and it will take time to rebuild it if I will switch to another thing.

JukkaL · 2018-02-20T12:36:15Z

Finally, type aliases depend on their expansion, aliases directly used, and type variables (for generic aliases). So, B = Dict[A, Cls] generates <A> -> , <Cls> -> 

What about generating dependencies like <A> -> m, <Cls> -> m instead, if the alias is defined in m? This way we'd get rid of one processing step:

A is changed and it triggers <A>.
We reprocess m, the module containing the type alias B.
AST diff of m notices that the alias m.B has changed, so we trigger <m.B>.
We reprocess each target that uses B in an annotation.

Also, do you test that changing A.__init__ causes calls to B to be reprocessed?

I prefer the second option, since all this picture of dependencies is now in my head and it will take time to rebuild it if I will switch to another thing.

Yeah, let's go with the second option (at least unless the symbol table refactoring turns out to be a simple thing).

ilevkivskyi · 2018-02-20T12:55:19Z

What about generating dependencies like <A> -> m, <Cls> -> m instead, if the alias is defined in m?

I think the performance win will be pretty minor, since currently we already have <A> ->  and  -> m. So I would prefer not to do this now (I think correctness/simplicity is more important now), unless you think it is something important.

Also, do you test that changing A.__init__ causes calls to B to be reprocessed?

Yes, there are some tests still missing, I have a TODO item for this one too.

JukkaL · 2018-02-20T13:37:13Z

I think the performance win will be pretty minor, since currently we already have <A> ->  and  -> m. So I would prefer not to do this now (I think correctness/simplicity is more important now), unless you think it is something important.

The performance win isn't really the only motivation for preferring minimal dependencies. I think that this is important due to a few reasons:

If we have non-minimal/optimal dependencies, it's harder to reason about bugs and correct behavior -- logs will have redundant steps, etc. This can result in developers tending to ignore suspicious-looking log data, since it's easy to just assume that it's because of known issue X, even if the logs are actually showing a previously unknown issue.
If we have non-minimal/optimal dependencies, bugs elsewhere in fine-grained mode may get unnoticed because of the redundant work.
It looks like we can reprocess everything that uses an alias B in an annotation twice: first after A changes, and second time after we've reprocessed m. The performance impact could be pretty bad in some cases.
Even if the performance impact of this is small, it might interact badly with other performance problems (say, if you have three issues that cause 2x extra work, you could suddenly have 8x extra work if they all happen at the same time).

ilevkivskyi · 2018-02-20T17:43:22Z

This is almost ready. I still need to figure out what best to do with builtins and typing and add tests for .__init__ deps.

JukkaL

Almost there! Just a bunch of minor things.

JukkaL · 2018-02-21T13:02:23Z

test-data/unit/diff.test

@@ -618,3 +618,53 @@ p = Point(dict(x=42, y=1337))
 [out]
 __main__.Point
 __main__.p
+
+[case testTypeAliasSimple]
+A = int


In all of these test cases, also include a case where there is no difference to make sure that we don't just always consider type aliases different. Example:

[case testTypeAliasSimple] A = int B = int [file next.py] A = str B = int [out] __main__.A

JukkaL · 2018-02-21T13:06:58Z

test-data/unit/fine-grained.test

+b.py:2: error: Incompatible types in assignment (expression has type "int", variable has type "str")
+
+[case testAliasFineChainedFunc]
+# This and next don't work without <A> -> <B> intermediate deps


Is this comment out of date?

JukkaL · 2018-02-21T13:13:46Z

test-data/unit/fine-grained.test

+        x: a.A = int()
+[out]
+==
+b.py:4: error: Incompatible types in assignment (expression has type "int", variable has type "str")


Potential new tests (if they don't already exist):

Create a type alias (initially there is a stale reference to a type alias).

Delete type alias (after update there is a stale reference to a type alias).

Switch a class to a type alias (or type alias to class).

JukkaL · 2018-02-21T13:16:12Z

test-data/unit/deps-types.test

+<m.x> -> m
+<a.A> -> m
+<a> -> m
+<mod.I> -> <m.x>, m


Can you also dump dependencies in a? I'd like to see the dependency <mod.I> -> a at least.

JukkaL · 2018-02-21T13:19:39Z

test-data/unit/deps-types.test

+A = I
+[file mod.py]
+class I: pass
+[out]


Again would also be good to see dependencies in a.

JukkaL · 2018-02-21T13:23:27Z

test-data/unit/deps-types.test

+[case testAliasDepsNestedMod]
+from mod import I, S, D
+A = D[S, I]
+B = D[S, A]


This test case would be more interesting if A and B would be defined in different modules, and if we dumped the dependencies within both modules.

JukkaL · 2018-02-21T13:24:02Z

test-data/unit/deps-types.test

+[case testAliasDepsNestedFunc]
+from mod import I, S, D
+A = D[S, I]
+B = D[S, A]


This is similar to above -- would be better to split across two modules.

JukkaL · 2018-02-21T13:32:56Z

test-data/unit/deps-types.test

+[case testAliasDepsRuntime]
+from mod import I, S, D
+A = I
+x = D[S, A]()


This would be more interesting if A would be defined in another module.

JukkaL · 2018-02-21T13:33:57Z

test-data/unit/deps-types.test

+<m.P> -> m.P
+<mod.I> -> <m.P.x>, <m.P>, m, m.P
+
+[case testAliasDepsTypedDict]


Also test functional definition of typed dict.

JukkaL · 2018-02-21T13:34:12Z

test-data/unit/deps-types.test

+<mod.I> -> <m.x>, m
+<mod.S> -> <m.x>, m
+
+[case testAliasDepsNamedTuple]


Also test functional definition of named tuple.

…ng tests

JukkaL

Thanks for updates -- I only have two remaining questions, otherwise looks good.

JukkaL · 2018-02-21T17:24:06Z

test-data/unit/deps-types.test

+class I: pass
+[builtins fixtures/dict.pyi]
+[out]
+<m.A> -> m


Like above, does this actually test that the dependency is generated, or would the dependency generate for just A = I, without the typed dict definition?

JukkaL · 2018-02-21T17:24:29Z

test-data/unit/deps-types.test

+[file mod.py]
+class I: pass
+[out]
+<m.A> -> m


Does this actually test that the dependency is generated, or would the dependency generate for just A = I, without the named tuple definition?

JukkaL

LGTM -- feel free to merge once the build is green.

* master: (27 commits) Don't call --strict-optional and --incremental experimental (python#4642) Sync typeshed (python#4641) Fix callable types with inconsistent argument counts (python#4611) Fix example (add 'class A:') Make psutil an optional dependency (python#4634) mypy and mypy_extensions aren't posix only (python#3765) Documentation for attr support (python#4632) Use read_with_python_encoding in stubgen to handle file encoding (python#3790) Sync typeshed (python#4631) Add remaining core team emails to CREDITS (python#4629) Fix issues with attr code. (python#4628) Better support for converter in attrs plugin. (python#4607) Clean up credits (python#4626) Support type aliases in fine-grained incremental mode (python#4525) Fine-grained: Fix crash caused by unreachable class (python#4613) Treat divmod like a binary operator (python#4585) Sync typeshed (python#4605) Fine-grained: Don't infer partial types from multiple targets (python#4553) Fine-grained: Compare symbol table snapshots when following dependencies (python#4598) Fix type of forward reference to a decorated class method (python#4486) ...

Fixes python#4394 This covers all common cases and majority of corner cases: *Simple aliases *Generic aliases *Forward references *Nested aliases *Chained aliases * Special forms Note that among tests added some also pass without this addition (probably because some dependencies are added by coincidence). Note that I mainly focus on false negatives, since in my experience while playing with fine-grained daemon, this is the most typical problem.

Ivan Levkivskyi added 7 commits January 30, 2018 22:15

Basic implementation

3eed2a5

Fix processing of aliases in base classes and of chained aliases; add…

3e895d6

… tests

Fix processing of aliases in base classes and of chained aliases; add…

8d42ca7

… tests

Revert "Fix processing of aliases in base classes and of chained alia…

c0a6ed7

…ses; add tests" This reverts commit 8d42ca7.

Skip two tests; minor fixes

54289f0

Refsactor dependency update in a method

0e365e8

3.5.1

8d88890

JukkaL reviewed Feb 1, 2018

View reviewed changes

Ivan Levkivskyi and others added 11 commits February 1, 2018 15:23

Review comments 1/3

e43821f

Review comments 2/3

2b25e91

Minor fixes, not done yet with review comments.

d5306f2

Update some tests as requested. More refactoring needed to record Typ…

fa6d426

…eVar fullname. Also maybe record exact location of type alias in semanal to fire attribute triggers.

Add more tests

d9b397e

Add type variables to type aliases dependencies

d7adb15

Fix typos

3a4b12b

Add few more dependencies

afa4967

Fix aststrip

d19455a

Fire module trigger if module symbol table changed

3984464

Fix lint

224d02d

Ivan Levkivskyi added 2 commits February 8, 2018 20:31

Add extra fired triger to the test

108a40d

Change the order of messages

b3a0488

Revert "Change the order of messages"

ee1fdc9

This reverts commit b3a0488.

ilevkivskyi closed this Feb 8, 2018

ilevkivskyi reopened this Feb 8, 2018

Merge remote-tracking branch 'upstream/master' into support-fine-aliases

811be9c

Add missing annotation

6e1cc7a

JukkaL reviewed Feb 19, 2018

View reviewed changes

Avoid long-range (indirect) dependencies for aliases

3b9b19e

Add some more deps tests with three modules

e5ab3eb

Ivan Levkivskyi added 4 commits February 20, 2018 13:42

Add AST diff tests for aliases

4832e37

Remove intermediate type alias deps

b08aef8

Replace full_target with target in alias deps

35f4380

Merge remote-tracking branch 'upstream/master' into support-fine-aliases

a027399

Ivan Levkivskyi added 5 commits February 20, 2018 21:26

Don't add deps like <Alias> -> <fun>, only <Alias> -> fun

8f3cb32

Add two more tests

e8e01c9

Fix test name and output; prepare for builtins problem investigation

b259da4

Merge remote-tracking branch 'upstream/master' into support-fine-aliases

1fb320c

Strip IndexExpr

6817ba5

JukkaL reviewed Feb 21, 2018

View reviewed changes

Ivan Levkivskyi added 3 commits February 21, 2018 15:40

Add extra dep from __init__ to alias definition scope and correspondi…

b8893f0

…ng tests

More deps tests; allow showing all deps

ef7f3b9

Add remaining required tests

86b1bfd

ilevkivskyi mentioned this pull request Feb 21, 2018

Fine-grained: Inconsistent behavior for nested NewType #4615

Open

JukkaL reviewed Feb 21, 2018

View reviewed changes

Improve two tests

0a4a23e

JukkaL approved these changes Feb 21, 2018

View reviewed changes

ilevkivskyi merged commit 8f253c2 into python:master Feb 21, 2018

ilevkivskyi deleted the support-fine-aliases branch February 21, 2018 17:57

		@@ -0,0 +1,80 @@
		"""Track current scope to easily calculate the corresponding fine-grained target.

		This is currently only used by mypy.semanal and mypy.server.deps.

		@@ -506,6 +537,13 @@ def visit_yield_from_expr(self, e: YieldFromExpr) -> None:

		# Helpers

		def add_type_alias_deps(self, target: str, add_trigger: bool = False) -> None:

Support type aliases in fine-grained incremental mode #4525

Support type aliases in fine-grained incremental mode #4525

Conversation

ilevkivskyi commented Jan 31, 2018

JukkaL 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Feb 1, 2018

ilevkivskyi commented Feb 1, 2018

ilevkivskyi commented Feb 8, 2018

ilevkivskyi commented Feb 8, 2018

ilevkivskyi commented Feb 8, 2018

JukkaL 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilevkivskyi commented Feb 19, 2018

JukkaL commented Feb 20, 2018

ilevkivskyi commented Feb 20, 2018

JukkaL commented Feb 20, 2018

ilevkivskyi commented Feb 20, 2018

JukkaL 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

Choose a reason for hiding this comment

JukkaL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JukkaL left a comment

Choose a reason for hiding this comment