ENH Add metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier #27308

StefanieSenger · 2023-09-06T20:11:59Z

Reference Issues/PRs

Towards #22893

What does this implement/fix? Explain your changes.

Adds metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier
The routing is added in the fit and partial_fit methods if they exist
A test is added for checking that the coefficients are different when different sample weights are passed. Another test asserts if the correct error message is shown if enable_metadata_routing is False, but metadata is passed.

Comments

I had do put those three together, because they share common functions.

Thanks @glemaitre for your help!

github-actions · 2023-09-06T20:13:56Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: b1a8003. Link to the linter CI: here}

sklearn/tests/test_multiclass.py

adrinjalali

need to add these classes to test_metaestimators_metadata_routing.py, and apply the same comments on the other instances in this PR.

Otherwise it's looking good.

sklearn/multiclass.py

sklearn/tests/test_multiclass.py

adrinjalali · 2023-09-07T11:15:01Z

sklearn/tests/test_multiclass.py

+        # TODO: should we raise an error if sample weight is requested but is not
+        # passed in the router's fit method when enable slep006 is True?
+        # clf.fit(iris.data, iris.target)


@glemaitre see #23920

OK I see, we can remove the comment then.

We would need to settle on the behaviour at some point. I am a bit on the same side as @thomasjpfan, finding that this is weird not to raise anything (I can be happy with a warning).

adrinjalali · 2023-09-07T11:15:15Z

sklearn/tests/test_multiclass.py

+    """Assert if coefficients differ when estimators are partial_fitted with different
+    `sample_weights`."""
+    sample_weight = np.ones_like(iris.target, dtype=np.float64)


same here, the common tests should test this

adrinjalali · 2023-09-07T11:16:01Z

sklearn/tests/test_multiclass.py

+    """Test that the right error message is raised when metadata params
+    are passed while `enable_metadata_routing=False`."""
+    sample_weight = np.ones_like(iris.target, dtype=np.float64)


not sure if we need this, this machinary is tested in the base class level.

Maybe, @glemaitre ?

Fine with removing the tests if it is tested at a "common" level. It was handy to have it to quickly develop.

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>

…o routing1

sklearn/tests/test_multiclass.py

StefanieSenger · 2023-09-07T18:44:16Z

sklearn/tests/test_multiclass.py

+        clf.partial_fit(
+            iris.data,
+            iris.target,
+            classes=np.unique(iris.target),
+            sample_weight=sample_weight,
+        )


Migrating this test towards sklearn/tests/test_metaestimators_metadata_routing.py isn't easily done.

Reason: When partial_fitting OneVsRestClassifier and OneVsOneClassifier an additional param (classes=np.unique(y)) needs to be passed, that the central tests don't account for (btw. I wonder why the function isn't internally doing that):

sklearn/utils/multiclass.py:410: ValueError: classes must be passed on the first call to partial_fit.

This error occurs when implementing the central test with routing_methods: partial_fit and the tests then call OneVsRestClassifier.partial_fit() without classes (as they are supposed to).

In test_setting_request_removes_error, should we add a condition checking for additional params before calling method(X, y, **kwargs)? But then, in the next step, the methods of the artificial estimators from sklearn/tests/metadata_routing_common.py (for instance ConsumingClassifier) need to be changed as well, so they would accept classes. I don't know how many meta-estimator's methods take additional params like classes, but wouldn't it be quite a lot?

this is a good one. We should certainly fix our common tests to handle cases when partial_fit accepts classes and when not. This could be done by a try/except catching TypeError. As for adding classes to other partial_fit, seems like a good idea, I wonder why they don't support it already.

Okay, I will try that.

But what is the reason why partial_fit allows the user to define classes?

I cannot think of any other way than classes=np.unique(y) to define classes. Why / in what case would a user want to deviate from that?

So, I made something for test_setting_request_on_sub_estimator_removes_error that looks like this:

try: method(X, y, **method_kwargs) except ValueError: method(X, y, classes=np.unique(y), **method_kwargs)

And ConsumingClassifier from sklearn/tests/metadata_routing_common.py now feeds classes=None in its partial_fit method.

It works at least. Please let me know if this is correct, @adrinjalali

I cannot think of any other way than classes=np.unique(y) to define classes. Why / in what case would a user want to deviate from that?

Cause the user might not have all the classes present in a batch of data passed to partial_fit. There can be a case where the batch being passed to partial_fit includes only a subset of total classes, and the classes are pre-defined by the user rather than selected from each batch.

I would do the code more as:

try: method(X, y, classes=np.unique(y), **method_kwargs) except TypeError: method(X, y, **method_kwargs)

I had to do it like this, because MultiOutputClassifier.partial_fit() expects its classes in another format.

try: method(X, y, classes=np.unique(y), **method_kwargs) except TypeError: method(X, y, **method_kwargs) except IndexError: method(X, y, **method_kwargs)

StefanieSenger · 2023-09-07T18:52:02Z

@adrinjalali, thank you for your review and the comments. :)

I've tried to migrate the tests to the central test file (sklearn/tests/test_metaestimators_metadata_routing.py), but due to unique characteristics of the multiclassifier's partial_fit methods and how OutputCodeClassifier creates its own sub-estimator in special cases, this is not possible without disruption.

I've tried to explain the reasons in the two comments and hope what I wrote is understandable.

Summary of what is done and not done:
Central test tests all the "fit" methods, but for OutputCodeClassifier not correctly, because it creates it's own sub_estimator sometimes.
Central test does NOT test "partial_fit", because it expects an additional param classes.

9E12

StefanieSenger · 2023-09-07T18:59:48Z

sklearn/tests/metadata_routing_common.py

@@ -60,7 +60,7 @@ def check_recorded_metadata(obj, method, split_params=tuple(), **kwargs):
        if key in split_params and recorded_value is not None:
            assert np.isin(recorded_value, value).all()
        else:
-            assert recorded_value is value
+            assert np.allclose(recorded_value, value)


I had to do this, because of a very slight mismatch when testing OneVsOneClassifier.fit() (because of floating-point precision?)

But as I now see in the CI this change has caused another test to fail (test_metadata_routing_for_column_transformer), so this is a catch. Should I revert that change? How else to deal with the failure, that I originally wanted to prevent?

could you please explain why this change was needed? the metadata shouldn't be touched by the meta-estimator, so there should be no numerical issues in terms of mismatch. This would only fix an issue if we were somehow creating a new object given the old one, but the whole old one, which might be the case if we're doing OVO on a binary classification case since we'd select the whole data.

The error you're getting in the CI now is:

E numpy.exceptions.DTypePromotionError: The DType <class 'numpy._FloatAbstractDType'> could not be promoted by <class 'numpy.dtypes.StrDType'>. This means that no common DType exists for the given inputs. For example they cannot be stored in a single array unless the dtype is `object`. The full list of DTypes is: (<class 'numpy.dtypes.StrDType'>, <class 'numpy._FloatAbstractDType'>)

It's because this is now trying to compare non-numerical arrays which it cannot.

@adrinjalali I had tried this, because when running test_setting_request_on_sub_estimator_removes_error I had gotten the following error:

if key in split_params and recorded_value is not None: assert np.isin(recorded_value, value).all() else: > assert recorded_value is value E AssertionError

Now I believe it's due to feeding only part of the data into the sub-estumator in _fit_ovo_binary. So only assert np.isin(recorded_value, value).all() should be checked, not assert recorded_value is value.

Setting "preserves_metadata": "subset", for ovo does this, but regarding your explanations here, I still don't understand, why partial_fit didn't raise the same issue, only fit did.

that makes sense since we're indexing the original array, and the indexing, even though it might select all rows, might be returning a new object (albeit a view to the old one), therefore is wouldn't work, and therefore we should set it as a subset in the tests. partial_fit might be not doing the sub-selection then.

Nice, "preserves_metadata": "subset", it is then. :)

From what I understood, partial_fit also does a sub-selection. Could you look at the code to tell me if I'm wrong?

yes it does. I'm not sure if the test passes then. But I wouldn't worry about it.

Yes, the test for partial_fit also passes assert recorded_value is value, which it shouldn't.

I will immediately stop worrying about this, but not before telling you that this odd test passing when it shouldn't had lead me totally off track while finding the reasons for fit not passing the same test.

The problem is, your code is so flexible, that it's both, hard to read and hard to document.

sklearn/multiclass.py

adrinjalali · 2023-09-11T12:16:28Z

sklearn/tests/metadata_routing_common.py

@@ -60,7 +60,7 @@ def check_recorded_metadata(obj, method, split_params=tuple(), **kwargs):
        if key in split_params and recorded_value is not None:
            assert np.isin(recorded_value, value).all()
        else:
-            assert recorded_value is value
+            assert np.allclose(recorded_value, value)


could you please explain why this change was needed? the metadata shouldn't be touched by the meta-estimator, so there should be no numerical issues in terms of mismatch. This would only fix an issue if we were somehow creating a new object given the old one, but the whole old one, which might be the case if we're doing OVO on a binary classification case since we'd select the whole data.

The error you're getting in the CI now is:

E numpy.exceptions.DTypePromotionError: The DType <class 'numpy._FloatAbstractDType'> could not be promoted by <class 'numpy.dtypes.StrDType'>. This means that no common DType exists for the given inputs. For example they cannot be stored in a single array unless the dtype is `object`. The full list of DTypes is: (<class 'numpy.dtypes.StrDType'>, <class 'numpy._FloatAbstractDType'>)

It's because this is now trying to compare non-numerical arrays which it cannot.

sklearn/tests/test_multiclass.py

adrinjalali · 2023-09-11T13:13:42Z

sklearn/tests/test_multiclass.py

+        clf.partial_fit(
+            iris.data,
+            iris.target,
+            classes=np.unique(iris.target),
+            sample_weight=sample_weight,
+        )


this is a good one. We should certainly fix our common tests to handle cases when partial_fit accepts classes and when not. This could be done by a try/except catching TypeError. As for adding classes to other partial_fit, seems like a good idea, I wonder why they don't support it already.

adrinjalali · 2023-09-19T11:07:27Z

doc/whats_new/v1.4.rst

+- |Enhancement| :class:`multiclass.OneVsRestClassifier`,
+  :class:`multiclass.OneVsOneClassifier` and
+  :class:`multiclass.OutputCodeClassifier` now support metadata routing.
+  :meth:`multiclass.OneVsRestClassifier.fit`,
+  :meth:`multiclass.OneVsOneClassifier.fit` and
+  :meth:`multiclass.OutputCodeClassifier.fit` now accept ``**fit_params`` which
+  are passed to the estimator used.
+  :meth:`multiclass.OneVsRestClassifier.partial_fit` and
+  :meth:`multiclass.OneVsOneClassifier.partial_fit` now accept
+  ``**partial_fit_params`` which are passed to the estimator used. :pr:`27308`
+  by :user:`Stefanie Senger <StefanieSenger>`.


All such entries are now placed on top of the file together, you can move this up one there as well. You might need to merge with main to see that.

@adrinjalali Do you mean #27421, which is not merged yet?

No I mean #27386 which is merged.

Found it, thanks. Is adding metadata routing to estimators |Enhancement| or |Feature|, btw? Right now both is used and I've ordered alphabetically.

We're gonna use |Feature| for the rest of them.

adrinjalali · 2023-09-19T14:56:58Z

sklearn/tests/metadata_routing_common.py

@@ -60,7 +60,7 @@ def check_recorded_metadata(obj, method, split_params=tuple(), **kwargs):
        if key in split_params and recorded_value is not None:
            assert np.isin(recorded_value, value).all()
        else:
-            assert recorded_value is value
+            assert np.allclose(recorded_value, value)


adrinjalali · 2023-09-19T15:01:11Z

sklearn/tests/test_metaestimators_metadata_routing.py

+            try:
+                method(X, y, **method_kwargs)
+            except ValueError:
+                method(X, y, classes=np.unique(y), **method_kwargs)


you could reverse the order here, and catch TypeError and always pass classes if it's in the signature.

@adrinjalali Thanks for the suggestion. Could you please explain the advantages of reversing the order? I don't see it, unfortunately.

My thought was to try what covers the most cases first to potentially save computational resources. Vastly most meta-estimators don't have classes implemented.

we're basically checking if the signature of the function supports classes or not, if yes, we pass it. If the signature doesn't support it, we get a TypeError which is standard in Python. Relying on the ValueError is something in our code instead of standard python machinery.

Relying on the ValueError is something in our code instead of standard python machinery.

I'm sorry, but I can't follow your reasoning and I'd like to learn and understand what you ask me to change. Why is turning it around to check the least common case first, better? Why is having TypeError more desirable than ValueError?

One is raised by Python machinery, one is raised by our code, and other estimator might raise a different error than ValueError. Here relying on python's machinery is much more resilient and reliable than relying on code that we write in our estimators.

This now makes sense to me. 💡 Thank you. I will change it with the next push later today.

sklearn/tests/test_multiclass.py

…putCodeClassifier doc

adrinjalali · 2023-09-22T15:18:09Z

@StefanieSenger please let me know when you want a new review, and summarize remaining points since it's hard to find the ongoing discussions in the comments at this point.

StefanieSenger · 2023-09-22T21:10:08Z

@adrinjalali It's ready for a second review. The implementation and the testing are both complete as far as I am aware of and the CI is now green.

I believe that the tests in test_multiclass can now be entirely deleted. But we were waiting for @glemaitre s approval, because (before I added the multiclassifiers to the central test file for routing) he had advised me to put them.

Apart from this, there are no open questions from my side, and in case you (or someone) wonders about something, I will try to explain my reasoning, so we can find out if it holds.

adrinjalali

I think the individual tests can now be removed since we're testing them in the common tests.

adrinjalali · 2023-09-25T08:33:03Z

doc/whats_new/v1.4.rst

+- |Enhancement| :class:`~compose.ColumnTransformer` now supports metadata routing
+  according to :ref:`metadata routing user guide <metadata_routing>`. :pr:`27005`
+  by `Adrin Jalali`_.
+
+- |Enhancement| :class:`linear_model.LogisticRegressionCV` now supports
+  metadata routing. :meth:`linear_model.LogisticRegressionCV.fit` now
+  accepts ``**params`` which are passed to the underlying splitter and
+  scorer. :meth:`linear_model.LogisticRegressionCV.score` now accepts
+  ``**score_params`` which are passed to the underlying scorer.
+  :pr:`26525` by :user:`Omar Salman <OmarManzoor>`.


these two entries are duplicates of bellow, not sure why they're repeated here.

Hm, I believe it got in on an automated merging main into my branch. Deleting it here.

doc/whats_new/v1.4.rst

sklearn/tests/metadata_routing_common.py

adrinjalali · 2023-09-25T08:53:09Z

sklearn/tests/test_metaestimators_metadata_routing.py

-y = rng.randint(0, 2, size=N)
+y = rng.randint(0, 3, size=N)


I would have two ys here, y and y_multi, since I'm not sure if all estimators support multiclass. We should pass y_multi only to the ones that support it.

All estimators support multiclass: https://scikit-learn.org/stable/modules/multiclass.html#multiclass-classification

This said, we also have a binary_only tag in the estimator.

@adrinjalali @glemaitre Please, could you tell me whether I should work on passing two different y's?

I have tried to find out about estimator tags and I couldn't find binary_only implemented anywhere except for in the tests.

In theory, you would have something like:

from sklearn.utils._tags import _safe_tags tags = _safe_tags(estimator) if tags["binary_onlty"]: estimator.fit(X, y_binary) else: estimator.fit(X, y_multiclass)

where y_binary and y_multiclass would be the created target at the beginning of the file.

So I foresee 2 solutions.

Create a ConsumerBinaryClassifier such that:

class ConsumerBinaryClassifier(ConsumerClassifier): def fit(self, X, y, **fit_params): y_type = type_of_target(y) if y_type != "binary": raise ValueError("This classifier supports only binary target") return super().fit(X, y, fit_params) def _more_tags(self): return {"binary_only": True}

Otherwise, we go from the principle that there are no binary classification and we just add a # TODO comment to modify the code if one day it happens.

@adrinjalali what do you think is best.

sklearn/tests/test_metaestimators_metadata_routing.py

adrinjalali · 2023-09-25T08:55:38Z

sklearn/tests/test_metaestimators_metadata_routing.py

+            except TypeError:
+                method(X, y, **method_kwargs)
+            except IndexError:
+                method(X, y, **method_kwargs)


this doesn't make sense, you're calling the method twice with the same args, why wouldn't it nor raise again?

It was, because MultiOutputClassifier.partial_fit() expects classes in another format. It didn't raise a TypeError, but an IndexError instead.

glemaitre

A couple of comments but it looks good already. Nice work @StefanieSenger @adrinjalali

sklearn/multiclass.py

glemaitre · 2023-09-25T12:27:42Z

sklearn/tests/test_multiclass.py

+    """Test that the right error message is raised when metadata params
+    are passed while `enable_metadata_routing=False`."""
+    sample_weight = np.ones_like(iris.target, dtype=np.float64)


Fine with removing the tests if it is tested at a "common" level. It was handy to have it to quickly develop.

sklearn/tests/metadata_routing_common.py

glemaitre · 2023-09-25T12:49:02Z

sklearn/tests/metadata_routing_common.py

+        if classes is not None:
+            self.classes_ = classes
+        else:
+            self.classes_ = np.unique(y)


This does not look like the pattern that we are using classifier. Usually, we will check that classes_ already exist (is or not the first call to partial_fit), if this is the first call then we create classes_ using classes that should not be None.

yeah but this is a test case, I don't mind having the right thing, but I don't think it matters.

@adrinjalali and @glemaitre Please let me know what you prefer and then I can try.

After talking with @adrinjalali, I have substitited this part with _check_partial_fit_first_call(self, classes), because this does everything required (check if classes_ exist, if not using classes for it) , also what is done partial_fit of MultiOutputClassifier, which then had to be fed with initial classes, so that it can pass this check.

glemaitre · 2023-09-25T12:51:28Z

sklearn/tests/test_metaestimators_metadata_routing.py

-y = rng.randint(0, 2, size=N)
+y = rng.randint(0, 3, size=N)


All estimators support multiclass: https://scikit-learn.org/stable/modules/multiclass.html#multiclass-classification

This said, we also have a binary_only tag in the estimator.

sklearn/tests/test_metaestimators_metadata_routing.py

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

…o routing1

adrinjalali

LGTM. Thanks @StefanieSenger

@glemaitre I think you can give it another review now.

glemaitre

LGTM on my side as well.

Kudos @StefanieSenger. This was not as straightforwards (as usual) with the partial_fit business.

…nd OutputCodeClassifier (scikit-learn#27308) Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com> Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

metadata routing for multiclass classifiers

7beb79b

StefanieSenger commented Sep 6, 2023

View reviewed changes

sklearn/tests/test_multiclass.py Outdated Show resolved Hide resolved

changelog added

7ef4e32

adrinjalali reviewed Sep 7, 2023

View reviewed changes

StefanieSenger and others added 4 commits September 7, 2023 17:43

Update sklearn/multiclass.py

ee6a69c

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>

little changes after review

dd56b18

Merge branch 'routing1' of github.com:StefanieSenger/scikit-learn int…

87f3312

…o routing1

test relocation, part 1

58f3234

StefanieSenger commented Sep 7, 2023

View reviewed changes

sklearn/tests/test_multiclass.py Outdated Show resolved Hide resolved

StefanieSenger commented Sep 7, 2023

View reviewed changes

adrinjalali reviewed Sep 11, 2023

View reviewed changes

StefanieSenger and others added 4 commits September 18, 2023 12:12

Merge branch 'main' into routing1

5e63f29

closing braket, still belonging to solve the merge conflict

125d34d

renaming routing_methods into estimator_routing_methods

f12b53e

partial_fit of multiclass-metaclasifiers and delete redundant test

b6944ef

adrinjalali reviewed Sep 19, 2023

View reviewed changes

StefanieSenger and others added 8 commits September 20, 2023 12:03

check only subsets of metadata is the same

275f53a

Merge branch 'main' into routing1

1c8e13d

fixed changelog

291218f

OutputCodeClassifier.fit variable-renaming and corrected error in Out…

a93e2c4

…putCodeClassifier doc

swap try except block

b9ffb40

Merge branch 'main' into routing1

d45e65b

variable name

2f8610c

init_args for CodeOutputClassifier

059f1bd

adrinjalali reviewed Sep 25, 2023

View reviewed changes

glemaitre self-requested a review September 25, 2023 09:23

extra_method_args

707bd4a

glemaitre reviewed Sep 25, 2023

View reviewed changes

adrinjalali mentioned this pull request Sep 25, 2023

FIX _partial_fit_binary passing classes only when classes are consumed #27464

Closed

StefanieSenger and others added 9 commits September 26, 2023 10:15

Update sklearn/multiclass.py

cc6622b

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

Update sklearn/multiclass.py

ac58a62

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

Merge branch 'main' into routing1

f44878c

little changes after review

cd21172

Merge branch 'routing1' of github.com:StefanieSenger/scikit-learn int…

8016eac

…o routing1

without RoutingNotSupportedMixin

de5369f

deleted multiclassifiers from UNSUPPORTED_ESTIMATORS

4bce619

ConsumingClassifier checks for existing classes

581d716

move to 'supported'

0fb5ba5

adrinjalali approved these changes Sep 29, 2023

View reviewed changes

adrinjalali mentioned this pull request Sep 29, 2023

SLEP006 - Metadata Routing task list #22893

Open

28 tasks

glemaitre self-requested a review October 2, 2023 13:43

Merge branch 'main' into routing1

b1a8003

glemaitre approved these changes Oct 2, 2023

View reviewed changes

glemaitre enabled auto-merge (squash) October 2, 2023 13:48

glemaitre merged commit 3260a93 into scikit-learn:main Oct 2, 2023

StefanieSenger mentioned this pull request Oct 5, 2023

FEA Add metadata routing to SelectFromModel #27490

Merged

StefanieSenger deleted the routing1 branch April 18, 2024 10:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH Add metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier #27308

ENH Add metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier #27308

ENH Add metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier #27308

ENH Add metadata routing to OneVsRestClassifier, OneVsOneClassifier and OutputCodeClassifier #27308

Conversation

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Comments

✔️ Linting Passed

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

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

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