1027 remove separation on primary secondary pipeline nodes #1031
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1031 +/- ##
==========================================
- Coverage 87.80% 87.64% -0.17%
==========================================
Files 208 210 +2
Lines 13970 14091 +121
==========================================
+ Hits 12267 12350 +83
- Misses 1703 1741 +38 ☔ View full report in Codecov by Sentry. |
maypink
force-pushed
the
1027-remove-separation-on-primary-secondary-pipeline-nodes
branch
from
53c1972
to
08cc2ce
Compare
fedot/core/optimisers/gp_comp/pipeline_composer_requirements.py
Outdated
Show resolved
Hide resolved
| @@ -295,13 +283,6 @@ def test_pipeline_with_primary_nodes_correct(): | |||
| assert has_primary_nodes(pipeline) | |||
|
|
|||
|
|
|||
| def test_pipeline_without_primary_nodes_raise_exception(): | |||
There was a problem hiding this comment.
Мне кажется, подобный тест будет актуален для мультимодальных пайплайнов, т.к. там есть смысл в primary node: через неё подаются данные в пайплайн (нода data_source_table и т.п.)
Мб его чуточку поменять?
There was a problem hiding this comment.
Поменять определенно стоит, потому что для обычных пайплайнов отсутствие primary ноды тоже не норма, а этот тест это покрывал
There was a problem hiding this comment.
Так вроде с новым подходом вроде вообще нет такой ситуации, как "отсутствие primary ноды". Можно только проверять, что конкретно находится в начальных PipelineNode
There was a problem hiding this comment.
кажется, что теперь отсутствие primary PipelineNode означает, что в пайплайне есть цикл, а это проверяется тестом ниже. убранный тест же покрывал именно проверку типов, а не по факту наличие/отсутствие чего-либо в nodes_from
There was a problem hiding this comment.
Имела в виду наличие is_primary хотя бы в одной ноде, но как хотите
|
|
||
|
|
||
| PrimaryNode = PipelineNode | ||
| SecondaryNode = PipelineNode |
There was a problem hiding this comment.
А это для чего здесь добавлено?
There was a problem hiding this comment.
| @@ -295,13 +283,6 @@ def test_pipeline_with_primary_nodes_correct(): | |||
| assert has_primary_nodes(pipeline) | |||
|
|
|||
|
|
|||
| def test_pipeline_without_primary_nodes_raise_exception(): | |||
There was a problem hiding this comment.
Поменять определенно стоит, потому что для обычных пайплайнов отсутствие primary ноды тоже не норма, а этот тест это покрывал
maypink
force-pushed
the
1027-remove-separation-on-primary-secondary-pipeline-nodes
branch
from
5d1f445
to
5fe9a25
Compare
maypink
deleted the
1027-remove-separation-on-primary-secondary-pipeline-nodes
branch
There is no more separation between
PrimaryNodeandSecondaryNode-- onlyPipelineNode. For nowPrimaryNodeandSecondaryNodeare just the same asPipelineNodefor backward compatibility.Determinion of node type can be done with
is_primarymethod.