[Refactor] Informative error messages on Scenario build failure

[JosuaCarl]

Description

The error message raise InvalidScenario(scenario.id) when scenario building fails is not very informative, because it leads to something like:

taipy.core.exceptions.exceptions.InvalidScenario: SCENARIO_MS_analysis_afc4e19b-be39-4651-bc9d-11f7807fd3af

with Traceback pointing only at the Scenario Factory.

I suggest adding a warning to

taipy/taipy/core/scenario/scenario.py

Lines 608 to 621 in 9cc59f2

	def _is_consistent(self) -> bool:
	"""Check if the scenario is consistent."""
	dag = self._build_dag()
	if dag.number_of_nodes() == 0:
	return True
	if not nx.is_directed_acyclic_graph(dag):
	return False
	for left_node, right_node in dag.edges:
	if (isinstance(left_node, DataNode) and isinstance(right_node, Task)) or (
	isinstance(left_node, Task) and isinstance(right_node, DataNode)
	):
	continue
	return False
	return True

like:

import warnings
warnings.warn( f"Graph directed: {nx.is_directed( dag )}, Cycle found at: {nx.cycles.find_cycle( dag )}" )

and

if not (isinstance(left_node, DataNode) and isinstance(right_node, Task)) and 
   not (isinstance(left_node, Task) and isinstance(right_node, DataNode) ):
    warnings.warn(f"Edge between left node:{left_node.get_label()} and right node:{right_node.get_label()} does not connect {Task} and {DataNode}")
    return False

Acceptance Criteria

• Any new code is covered by a unit tested.
• Check code coverage is at least 90%.

Code of Conduct

• I have checked the existing issues.
• I am willing to work on this issue (optional)

[jrobinAV]

Hello @JosuaCarl

Thank you for your issue. It's a great idea. Would you like to be assigned?

A few minor remarks:

1. In Taipy, we handle logs with a logger that you can retrieve using the following code:

   from taipy.common.logger._taipy_logger import _TaipyLogger
   
   _logger = _TaipyLogger._get_logger()
   
   _logger.error("My error message!")
   _logger.warning("My warning.")
   _logger.info("My info")

2. They should be error messages and not warnings. Especially because we raise an exception in case it is inconsistent.

3. We have a similar but slightly different piece of code for sequences. It should be covered by the issue as well.

   taipy/taipy/core/sequence/sequence.py

   Lines 341 to 355 in 9cc59f2

   	def _is_consistent(self) -> bool:
   	dag = self._build_dag()
   	if dag.number_of_nodes() == 0:
   	return True
   	if not nx.is_directed_acyclic_graph(dag):
   	return False
   	if not nx.is_weakly_connected(dag):
   	return False
   	for left_node, right_node in dag.edges:
   	if (isinstance(left_node, DataNode) and isinstance(right_node, Task)) or (
   	isinstance(left_node, Task) and isinstance(right_node, DataNode)
   	):
   	continue
   	return False
   	return True

[JosuaCarl]

Yes, I would like to be assigned. Although I am not sure how much I will be able to do until the new year.

[jrobinAV]

Yes, I would like to be assigned. Although I am not sure how much I will be able to do until the new year.

Thanks a lot! Don't worry, we are not in a rush for this issue.

[JosuaCarl]

Small question before I start:
The purpose of the _is_constent method is to evaluate consistency and return it. I suggested a warning, in order to enable the code to finish and pass on a boolean value (which is expected by following methods). Should I refactor the methods, that rely on _is_constent, such that an error is watched and raised later , resulting in equation to False or should the _is_constent method itself serve as the main Error handler, such that it can called without expecting a return and the code simply continues on, when no Error is raised ?

[jrobinAV]

That's a good question.

I would keep the current behavior for both is_consistent methods. They should still return a boolean. The only difference would be that they log a meaningful error before returning False.
And we let the responsibility of raising an exception or not to the piece of code that calls the is_consistent method.

[github-actions]

This issue has been labelled as "🥶Waiting for contributor" because it has been inactive for more than 14 days. If you would like to continue working on this issue, please add another comment or create a PR that links to this issue. If a PR has already been created which refers to this issue, then you should explicitly mention this issue in the relevant PR. Otherwise, you will be unassigned in 14 days. For more information please refer to the contributing guidelines.