Review #2

Olivier998 · 2025-07-29T18:05:01Z

Review des fichiers dans le folder MED3pa/MED3pa/*.

MED3pa/datasets/loading_context.py

+    Returns:
+        List[str]: A list of supported file formats.
+    """
+    return list(DataLoadingContext.strategies.keys())


MED3pa/datasets/manager.py

+        elif dataset_type == 'testing':
+            self.testing_set = dataset
+        else:
+            raise ValueError(f"Invalid dataset_type provided: {dataset_type}")


MED3pa/med3pa/mdr.py

+            elif operator == '!=':
+                mask &= x[:, column_index] != value
+            else:
+                raise ValueError(f"Unsupported operator '{operator}' in condition '{condition}'.")


MED3pa/med3pa/profiles.py

+
+        Args:
+            profiles_list (List[dict]): List of profiles data.
+            to_dict (bool, optional): If True, transforms profiles to dictionaries. Defaults to True.


mariemkallel16 · 2025-07-30T13:17:09Z

MED3pa/models/base.py

+
+        return {
+            "model": self.__baseModel.__class__.__name__,
+            "model_type": self.__baseModel.__class__.__name__,


je trouve que model et model_type sont redondants, peut être une seule variable à séparer plus tard?

MED3pa/models/concrete_regressors.py

+        Args:
+            base_model (RandomForestRegressorModel): A prototype instance of RandomForestRegressorModel.
+            n_models (int): The number of RandomForestRegressorModel instances in the ensemble.
+            params (Dict[str, Any]): A list of parameter dictionaries for each model in the ensemble.


MED3pa/models/data_strategies.py

@@ -0,0 +1,148 @@
+"""
+This module is crucial for data handling, utilizing the **Strategy design pattern** and therefor offering multiple strategies to transform raw data into formats that enhance model training and evaluation.


MED3pa/visualization/profiles_visualization.py

+    Args:
+        result (Med3paResults): The results of the experiment to visualize.
+        dr (int): Declaration rate of predictions for the visualization.
+        samp_ratio (int): The minimum samples ratio in each profile (in percentage).


MED3pa/models/regression_metrics.py

+            'R2': cls.r2_score
+        }
+        if metric_name == '':
+            return list(metrics_mappings.keys())


MED3pa/models/concrete_regressors.py

+        Initializes the EnsembleRandomForestRegressorModel with multiple RandomForestRegressor models.
+
+        Args:
+            base_model (RandomForestRegressorModel): A prototype instance of RandomForestRegressorModel.


ouaelesi · 2025-07-30T16:54:05Z

MED3pa/med3pa/experiment.py

La methode _run_by_set est trop longue ( 225 lignes God Function ), je pense c bien de diviser la méthode en plusieurs sous-méthodes pour réspecter le principe de SRP

MED3pa/med3pa/models.py

MED3pa/med3pa/uncertainty.py

LaribiHakima · 2025-07-30T16:18:32Z

MED3pa/datasets/loading_context.py

+    def get_strategy(self) -> DataLoadingStrategy:
+        """
+        Returns the currently selected data loading strategy.
+
+        Returns:
+            DataLoadingStrategy: The currently selected data loading strategy.
+        """
+        return self.selected_strategy


ça devrait pas être une properety ?

LaribiHakima · 2025-07-30T17:29:13Z

MED3pa/datasets/loading_context.py

@@ -0,0 +1,78 @@
+"""
+This module provides a flexible framework for loading datasets from various file formats by utilizing the **strategy design pattern**.


tu pourrais ajouter le nom de l'auteur dans l'entête de chaque fichier

MED3pa Team!

LaribiHakima · 2025-07-30T17:34:29Z

MED3pa/datasets/loading_strategies.py

+from abc import ABC, abstractmethod
+
+
+class DataLoadingStrategy(ABC):


Quel est le but de créer une classe parent qui est héritée par une seule classe seulement, et qui ne contient qu'une seule méthode statique ? est-ce que créer une fonction load_data_from_csv() n'est pas suffisant ?

LaribiHakima · 2025-07-30T17:37:58Z

MED3pa/datasets/manager.py

+        if dataset_type == 'training':
+            self.base_model_training_set = dataset
+        elif dataset_type == 'validation':
+            self.base_model_validation_set = dataset
+        elif dataset_type == 'reference':
+            self.reference_set = dataset
+        elif dataset_type == 'testing':
+            self.testing_set = dataset
+        else:
+            raise ValueError(f"Invalid dataset_type provided: {dataset_type}")


ce bloc de codes est répété plusieurs fois, tu peux en faire une méthode

MED3pa/datasets/manager.py

+        if self.base_model_training_set is not None:
+            self.base_model_training_set.column_labels = columns
+        if self.base_model_validation_set is not None:
+            self.base_model_validation_set.column_labels = columns
+        if self.reference_set is not None:
+            self.reference_set.column_labels = columns
+        if self.testing_set is not None:
+            self.testing_set.column_labels = columns


LaribiHakima · 2025-07-30T18:26:13Z

MED3pa/med3pa/results.py

+        if mode == 'ipc':
+            self.ipc_scores = scores
+        elif mode == "apc":
+            self.apc_scores = scores
+        elif mode == "mpc":
+            self.mpc_scores = scores


pourraient tous être des setters

MED3pa/med3pa/uncertainty.py

+    Concrete implementation of the UncertaintyMetric class using Sigmoidal error.
+    """
+    @staticmethod
+    def calculate(x: np.ndarray, predicted_prob: np.ndarray, y_true: np.ndarray, threshold=0.5) -> np.ndarray:


LaribiHakima · 2025-07-30T18:30:22Z

MED3pa/models/abstract_models.py

+    def is_pickled(self) -> bool:
+        """
+        Returns whether the model has been loaded from a pickled file.
+
+        Returns:
+            Boolean: has the model been loaded from a pickled file.
+        """
+        return self.pickled_model


MED3pa/models/concrete_regressors.py

+        # if params is not None:
+        #     if training_parameters is None:
+        #         training_parameters = params
+        #     else:
+        #         training_parameters.update(params)


LaribiHakima · 2025-07-30T18:44:48Z

MED3pa/models/factories.py

+from .concrete_classifiers import XGBoostModel
+
+
+class ModelFactory:


une classe dont toutes les méthodes sont statiques ?

MED3pa/med3pa/experiment.py

+
+        if predicted_probabilities is None:
+            # base_model = base_model_manager.get_instance()
+            predicted_probabilities = base_model_manager.predict_proba(x)[:, 1]  # base_model.predict(x, True)


Olivier998 · 2025-08-20T00:13:59Z

MED3pa/models/concrete_classifiers.py

+from MED3pa.models.xgboost_params import valid_xgboost_custom_params, valid_xgboost_params
+
+
+class XGBoostModel(ClassificationModel):


à enlever ou refaire

lyna1404 and others added 30 commits May 28, 2024 15:59

initial commit

38d38e2

Implemented datasets module with tests

e60307a

added init files to det3pa and tests folders

98f6638

added environment.yml file

3a85989

updated environment.yml

723a175

added Github Actions CI workflow

76bf938

updated Github Actions ci workflow

5f3a3c1

updated Github Actions ci workflow

922db3d

updated environment.yml

4568daf

updated Github Actions ci workflow

c99c792

updated Github Actions ci workflow

897e146

implemented models subpackage along with its unittests

546aa5f

finalizing the package docs

b44f1ec

package documentation

21b3bcb

Package code review

d7a26e1

updated ci.yml

de02352

updated ci.yml

80815d4

Add : .gitignore file

2a25e46

Add : experiments

5e386c6

took into consideration code review comments

3f610c2

last fixes

be4df36

added readme file

fe93583

updated readme file

d1b460c

updated ci workflow

7d07a68

Create LICENSE

aca8512

Update LICENSE

e620fcf

readthedocs publication

400b60d

readthedocs.yml and setup.py added

34537ca

Merge branch 'main' of https://github.com/lyna1404/det3pa

86cebdd

update readthedocs requirements file

c8d9f97

Olivier998 added 12 commits January 1, 2025 20:48

test

ff8f121

Added preprocessing on args for checkpointing

fac52c6

remove weight balance when 1 class

93ee86f

removed ray from ipc optimize

1d89926

main checkpointer format to pickle

7977af9

added ray shutdown to allow resources redistribution

8cc17d9

modifs review

6f82348

readme correction

d35a75b

changed readme

0d39064

modifs pour review

6f925d2

corrected links

1bc0746

Merge branch 'code_review' into review

d3562c3

Olivier998 assigned LaribiHakima, Olivier998, ouaelesi and mariemkallel16 Jul 29, 2025

mariemkallel16 reviewed Jul 30, 2025

View reviewed changes

ouaelesi reviewed Jul 30, 2025

View reviewed changes

MED3pa/med3pa/models.py

This comment was marked as resolved.

Sign in to view

This comment was marked as resolved.

Sign in to view

ouaelesi reviewed Jul 30, 2025

View reviewed changes

MED3pa/med3pa/uncertainty.py

This comment was marked as resolved.

Sign in to view

LaribiHakima reviewed Jul 30, 2025

View reviewed changes

Olivier998 commented Jul 30, 2025

View reviewed changes

MED3pa/med3pa/experiment.py Outdated

if predicted_probabilities is None:

# base_model = base_model_manager.get_instance()

predicted_probabilities = base_model_manager.predict_proba(x)[:, 1] # base_model.predict(x, True)

This comment was marked as resolved.

Sign in to view

removed .keys for dict to list conversion

97e456e

Olivier998 commented Aug 20, 2025

View reviewed changes

Olivier998 added 6 commits August 19, 2025 20:30

modifs code review

9326792

saving model instance rather than sub-instance

72233c7

added MED3pa usage example

a4367ca

removed deprecated folders

1fda1f2

updated readme

395c1ce

updated readthedocs

6bd16c5

		@@ -0,0 +1,148 @@
		"""
		This module is crucial for data handling, utilizing the Strategy design pattern and therefor offering multiple strategies to transform raw data into formats that enhance model training and evaluation.

		@@ -0,0 +1,78 @@
		"""
		This module provides a flexible framework for loading datasets from various file formats by utilizing the strategy design pattern.

		from abc import ABC, abstractmethod


		class DataLoadingStrategy(ABC):

		from .concrete_classifiers import XGBoostModel


		class ModelFactory:

		from MED3pa.models.xgboost_params import valid_xgboost_custom_params, valid_xgboost_params


		class XGBoostModel(ClassificationModel):

Review #2

Are you sure you want to change the base?

Review #2

Uh oh!

Conversation

Olivier998 commented Jul 29, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants