Fix params setting #1121
Fix params setting #1121
Conversation
|
Hello @YamLyubov! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2023-07-28 11:11:25 UTC |
Codecov Report
@@ Coverage Diff @@
## master #1121 +/- ##
==========================================
+ Coverage 78.54% 78.57% +0.02%
==========================================
Files 131 131
Lines 9323 9321 -2
==========================================
+ Hits 7323 7324 +1
+ Misses 2000 1997 -3
|
| @pytest.mark.parametrize('params, expected_params', [(DEFAULT_PARAMS_STUB, {"family": "gaussian", "link": "identity"}), | ||
| ({}, {"family": "gaussian", "link": "identity"}), | ||
| ({NESTED_PARAMS_LABEL: {'family': 'gaussian', 'link': 'log'}}, | ||
| {'family': 'gaussian', 'link': 'log'}), | ||
| ({'family': 'inverse_gaussian', 'link': 'inverse_power'}, | ||
| {'family': 'inverse_gaussian', 'link': 'inverse_power'})]) | ||
| def test_init_and_set_pipeline_node_with_params(params, expected_params): | ||
| node = PipelineNode(content={'name': 'glm', 'params': params}) | ||
| assert node.parameters == expected_params | ||
|
|
||
| node = PipelineNode('glm') | ||
| node.parameters = params | ||
| assert node.parameters == expected_params |
There was a problem hiding this comment.
А есть какой-нибудь тест с реальным тюнингом на вложенные параметры?
There was a problem hiding this comment.
Да, тест есть . Подправила его, потому что он предыдущую ошибку не находил)
| self._parameters = OperationParameters.from_operation_type(operation.operation_type, **params) | ||
| super().__init__(content={'name': operation, | ||
| 'params': self._parameters.to_dict()}, nodes_from=nodes_from) |
There was a problem hiding this comment.
Не очень понял, в чем тут проблема. Вроде и там и там параметры передаются без изменений. В чем тонкость?
There was a problem hiding this comment.
Как выше написала смысл в том, чтобы пропустить поступающие параметры через parameters.setter, который верно обработает параметры
There was a problem hiding this comment.
Теперь понял. Оставь, пожалуйста, коммент о необходимости setter, а то из кода это неочевидно
YamLyubov
force-pushed
the
fix-nested-params
branch
from
e47c7b3
to
15c2d97
Compare
* Fix params setting * Minor * PEP 8 * Correct test * Add comment
Setting of 'nested_space' parameters for node while tuning was not working correctly. While tuning new parameters were at first set for
OptGraphand thenOptGraphwas restored toPipeline. While restoring, pipeline nodes were created from scratch and new parameters were passed through the constructor of theNodeclass. That is why 'nested_space' parameters were not processed as they should.Also
examples/simple/multitask_classification_regression_api.pywas removed as the functionality is no longer supported.