chore/SOF 7921 1#149
Conversation
VsevolodX
commented
Jun 3, 2026
VsevolodX
commented
- update: adjust {} -> []
- update: tests:
- update: tests
- update: restructure unit context
- update: model serialization
- chore: mode
- update: unit has no context
- update: possible type
- update: execution to have context
- update: adjsut convergence
- update: use context item
- chore: cleanup
- update: execution unit test
- update: check for execution unit
- chore: simplify
- update: context to match NBs
- update: validate context item
- update: use standata for tests
- update: context shape in tests
- update: preserve yield data
| @property | ||
| def unit_context(self) -> Dict[str, Any]: | ||
| return self._points_grid_context( | ||
| yielded = self._points_grid_context( |
There was a problem hiding this comment.
Let's update _points_grid_context instead?
| from .execution_unit_input import ExecutionUnitInput | ||
| from .unit import Unit | ||
|
|
||
| Context = List[ContextItemSchema] |
There was a problem hiding this comment.
Why not just take from schema?
There was a problem hiding this comment.
It's an alias for ease of reading. Also if we eventually create Context class -- it is already used as such
| if isinstance(item, dict): | ||
| return item.get("name") | ||
| name = getattr(item, "name", None) | ||
| return str(name) if name is not None else None |
There was a problem hiding this comment.
Can't be none and should always just be item.get("name")
| def _coerce_context(cls, value: Any) -> Any: | ||
| if value is None: | ||
| return [] | ||
| return value |
| def get_context_item(self, name: str) -> Optional[Dict[str, Any]]: | ||
| for item in self.context: | ||
| if self._context_item_name(item) == name: | ||
| return item if isinstance(item, dict) else item.model_dump() |
| else: | ||
| self.context.append(item) | ||
|
|
||
| def get_context(self, name: str, default: Any = None) -> Any: |
There was a problem hiding this comment.
get_context_item_by_name
default should be {} if not present
There was a problem hiding this comment.
it is {} by default. by in Py we get "argument value is mutable", so we need to do the if None -> {} thing
| yield unit | ||
|
|
||
|
|
||
| def _assert_subworkflow_models_have_functional(workflow_payload, expected_functional): |
There was a problem hiding this comment.
This should be tested in mode, not here
| assert model.get("functional") == expected_functional | ||
|
|
||
|
|
||
| def _assert_execution_unit_context_is_webapp_shaped(unit): |
There was a problem hiding this comment.
This doesn't make sense - mention of webapp??
There was a problem hiding this comment.
I'll remove. This came from a file checkout.
VsevolodX
force-pushed
the
chore/SOF-7921-1
branch
from
59d88a4 to
f232508
Compare
| from mat3ra.ade.context.context_provider import ContextProvider as AdeContextProvider | ||
|
|
||
|
|
||
| class ContextProvider(AdeContextProvider): |
There was a problem hiding this comment.
TODO: remove context provider from Ade
| context=unit_for_convergence.context | ||
| ).get_reciprocal_vector_ratios() | ||
| kgrid_item = unit_for_convergence.get_context_item("kgrid") | ||
| provider_context = {"kgrid": kgrid_item["data"]} if kgrid_item else None |
There was a problem hiding this comment.
should be unit_for_convergence.get_context_item_by_name("kgrid")
| ).get_reciprocal_vector_ratios() | ||
| kgrid_item = unit_for_convergence.get_context_item("kgrid") | ||
| provider_context = {"kgrid": kgrid_item["data"]} if kgrid_item else None | ||
| reciprocal_vector_ratios = PointsGridDataProvider().get_reciprocal_vector_ratios( |
There was a problem hiding this comment.
Should be just PointsGridDataProvider(**provider_context)
| provider_context = {"kgrid": kgrid_item["data"]} if kgrid_item else None | ||
| reciprocal_vector_ratios = PointsGridDataProvider().get_reciprocal_vector_ratios( | ||
| context=provider_context, | ||
| ) |
There was a problem hiding this comment.
get_reciprocal_vector_ratios()
| pattern, f"{parameter_name} = {scope_reference}", input_name=input_name | ||
| ) | ||
| execution_unit.add_context({parameter_name: parameter_initial}) | ||
| execution_unit.add_context({"name": parameter_name, "data": parameter_initial}) |
There was a problem hiding this comment.
@k0stik - Will it work like this? The context would have {"name": "N_k", ...}, will the substitution work for the template based on the variable name?
| WORKFLOW_STANDATA = WorkflowStandata() | ||
|
|
||
|
|
||
| def execution_unit_config(application: str, workflow_name: str, unit_name: str) -> Dict[str, Any]: |
There was a problem hiding this comment.
get_execution_unit_config_by_application_workflow_unit
| assert wf.hash == expected_hash | ||
|
|
||
|
|
||
| def test_workflow_to_dict_is_json_serializable_after_model_assignment(): |
| self, dimensions: List[str], reciprocal_vector_ratios: Optional[List[float]] = None | ||
| ) -> Dict[str, Any]: | ||
| return PointsGridDataProvider().yield_data_with_overrides( | ||
| provider = PointsGridDataProvider(isEdited=True) |
There was a problem hiding this comment.
Why does it need to be True here?
There was a problem hiding this comment.
We replace dimensions with Jinja vars, so it means that we edit it by definition.
There was a problem hiding this comment.
We'll address all of this when removing CP from Ade and syncing py with JS
2 participants
