comparaison by ferhatbe · Pull Request #2 · ferhatbe/my_python_tp

ferhatbe · 2026-07-27T13:47:22Z

No description provided.

Delete calculator.py

…on_tp into feat/calculator

ferhatbe

Revue

Problèmes

`calculator.py`

Gestion d'exception dangereuse (ligne 7) : except: sans spécification du type d'exception est une mauvaise pratique. Cela capture toutes les exceptions, y compris KeyboardInterrupt et SystemExit.
Exception silencieuse : L'exception est capturée mais rien n'est retourné ni loggé, rendant le debugging impossible.
Manque d'espaces PEP8 (ligne 10) : additionner(a,b) devrait être additionner(a, b).
Nommage de variable peu clair (ligne 11) : r devrait avoir un nom plus explicite comme resultat.
Manque d'espaces autour des opérateurs (ligne 11) : r=a+b devrait être r = a + b.
Boucle non pythonique (lignes 14-16) : for i in range(len(items)) devrait être remplacé par for item in items.
Absence de docstrings : Aucune fonction n'a de documentation.
Pas de newline en fin de fichier : Le fichier devrait se terminer par une ligne vide (PEP8).
Print dans la logique métier (ligne 4) : Le print() devrait être évité dans une fonction de calcul.

`acte1.py`

Import de module inexistant (ligne 9) : langchain_classic.agents n'existe pas. Le module correct est langchain.agents.
Manque de gestion d'erreur : Aucune validation si les variables d'environnement sont présentes.
Pas de newline en fin de fichier : Le fichier devrait se terminer par une ligne vide.

Suggestions

Pour `calculator.py`

def diviser(x: float, y: float) -> float | None:
    """Divise x par y et retourne le résultat."""
    try:
        resultat = x / y
        return resultat
    except ZeroDivisionError:
        print(f"Erreur : division par zéro")
        return None

def additionner(a: float, b: float) -> float:
    """Additionne deux nombres."""
    resultat = a + b
    return resultat

def traiter_liste(items: list) -> None:
    """Affiche chaque élément de la liste."""
    for item in items:
        print(item)

Pour `acte1.py`

Corriger l'import : from langchain.agents import AgentExecutor, create_tool_calling_agent
Ajouter des vérifications :

if not all([GITHUB_TOKEN, GITHUB_REPO, os.environ.get("GITHUB_PR_NUMBER")]):
    raise ValueError("Variables d'environnement manquantes")

Verdict

❌ Changements requis - Le code contient des erreurs critiques (import cassé, gestion d'exceptions dangereuse) et de nombreuses violations PEP8. Merci de corriger avant merge.

ferhatbe

🔍 Revue de code - PR #2

✅ Points positifs

acte1.py : Bonne structure d'agent LangChain avec tools GitHub
Utilisation correcte de dotenv pour la configuration
Documentation des fonctions avec docstrings

⚠️ Problèmes identifiés

calculator.py (fichier de test)

Gestion d'erreur dangereuse (ligne 7) : except: sans spécifier l'exception et sans logging
Formatage PEP8 (ligne 10) : Manque d'espaces autour des paramètres additionner(a,b)
Variable non descriptive (ligne 11) : r devrait être result ou somme
Anti-pattern (ligne 15) : Utilisation de range(len()) au lieu d'itération directe
Newline manquante : Le fichier devrait se terminer par une ligne vide

acte1.py

Import non standard (ligne 9) : langchain_classic semble être une coquille (devrait être langchain)
Gestion d'erreur minimaliste : Les fonctions tools ne gèrent pas tous les cas d'erreur possibles

💡 Suggestions

Spécifier les exceptions (except ZeroDivisionError:)
Ajouter du logging pour le débogage
Utiliser les itérateurs Python (for item in items:)
Respecter PEP8 pour la lisibilité

🎯 Verdict

Changements requis avant merge. Les problèmes de calculator.py doivent être corrigés pour respecter les standards de qualité Python.

ferhatbe

Suggestion sur calculator.py:7

ferhatbe · 2026-07-28T09:25:50Z

+        resultat = x / y
+        print(resultat)
+        return resultat
+    except:


🚨 Problème critique : Utilisation d'un except: nu (bare except) qui capture toutes les exceptions, même les KeyboardInterrupt et SystemExit.

Pourquoi c'est problématique :

Masque les erreurs inattendues

Rend le débogage impossible

Viole PEP8 (E722)

Solution : Spécifier l'exception attendue (ZeroDivisionError) et gérer proprement le cas d'erreur avec un message explicite et un retour cohérent.

Suggested change

except:

except ZeroDivisionError:

print("Erreur : division par zéro")

return None

ferhatbe

🔍 Revue de code - PR #2

Merci @ferhatbe pour cette contribution ! Voici mon analyse détaillée :

✅ Points positifs

acte1.py : Architecture agent LangChain bien structurée avec tools GitHub
Bonne séparation des responsabilités (configuration, tools, agent, exécution)
Utilisation appropriée de AgentExecutor avec return_intermediate_steps
Documentation claire des fonctions avec docstrings

⚠️ Problèmes identifiés

calculator.py (fichier de test)

Ligne 7 : except: nu sans spécification d'exception - anti-pattern dangereux
Ligne 10 : Manque d'espaces autour des paramètres (a,b au lieu de a, b)
Ligne 11 : Variable r peu descriptive
Lignes 15-16 : Utilisation de range(len()) - anti-pattern Python (préférer itération directe)
Ligne 4 : print() dans une fonction métier (side effect)

acte1.py

Ligne 9 : Import langchain_classic.agents - ce module n'existe pas dans les versions récentes, devrait être langchain.agents
Ligne 56 : Pas de gestion d'erreur si OPENROUTER_API_KEY est absent
Ligne 28 : Manque de vérification du format de la réponse JSON avant accès aux clés

🎯 Suggestions d'amélioration

Ajouter des types hints sur toutes les fonctions
Implémenter une gestion d'erreur robuste avec des exceptions spécifiques
Ajouter des tests unitaires
Utiliser des loggers au lieu de print()
Valider les variables d'environnement au démarrage

📊 Verdict

Code fonctionnel mais nécessite des corrections sur les bonnes pratiques Python (PEP8, gestion d'erreurs). Je vais poster une suggestion inline pour la correction la plus critique.

ferhatbe

Suggestion sur calculator.py:7

ferhatbe · 2026-07-28T09:40:44Z

+        resultat = x / y
+        print(resultat)
+        return resultat
+    except:


⚠️ Exception nue détectée : Utiliser except: sans spécifier le type d'exception est un anti-pattern dangereux qui peut masquer des erreurs inattendues (KeyboardInterrupt, SystemExit, etc.).

Corrections proposées :

Spécifier ZeroDivisionError explicitement

Retourner None au lieu de pass silencieux

Supprimer le print() (ligne 5) pour éviter les side effects

Alternative avec logging :

except ZeroDivisionError as e: logger.warning(f"Division par zéro : {x}/{y}") return None

Suggested change

except:

except ZeroDivisionError:

return None

ferhatbe

Suggestion sur calculator.py:15

ferhatbe · 2026-07-28T09:40:50Z

+    return r
+
+def traiter_liste(items):
+    for i in range(len(items)):


🐍 Anti-pattern Python : Utiliser range(len(items)) est un code smell en Python.

Pourquoi c'est problématique :

Moins lisible (indirection inutile)

Risque d'IndexError si manipulation incorrecte

Non pythonique (PEP20 : "Simple is better than complex")

Solution proposée :
Itérer directement sur les éléments avec for item in items:

Si besoin de l'index :

for i, item in enumerate(items): print(f"{i}: {item}")

Suggested change

for i in range(len(items)):

for item in items:

ferhatbe

Revue

Problèmes

calculator.py

Ligne 7 : except: - Clause except trop large sans type d'exception spécifié. Cela masque toutes les erreurs, même critiques (KeyboardInterrupt, SystemExit). Spécifiez ZeroDivisionError ou au minimum Exception.

Ligne 7 : Aucun retour ni message d'erreur dans le bloc except. La fonction retourne implicitement None, ce qui peut causer des bugs silencieux.

Ligne 4 : print(resultat) - Utilisation de print dans une fonction de calcul. Les fonctions utilitaires ne devraient pas avoir d'effets de bord (logging/affichage). Laissez l'appelant gérer l'affichage.

Ligne 10 : def additionner(a,b): - Manque d'espace après les virgules (PEP8). Devrait être a, b.

Ligne 11 : Nom de variable non descriptif r. Utilisez resultat ou somme pour améliorer la lisibilité.

Lignes 14-16 : Fonction traiter_liste utilise range(len(items)) au lieu d'itérer directement. C'est un anti-pattern Python. Utilisez for item in items:.

Ligne 16 : Pas de retour à la ligne finale (PEP8 recommande un newline en fin de fichier).

Général : Aucune docstring sur les fonctions. Ajoutez des descriptions avec types de paramètres et valeurs de retour.

acte1.py

Ligne 9 : Import de langchain_classic.agents - module inexistant. Devrait probablement être langchain.agents.

Ligne 93 : Variable nommée résultat avec accent. Préférez l'anglais (result) ou sans accent (resultat) pour la compatibilité et les conventions.

acte2.py

Ligne 52-56 : La docstring de suggest_fix mentionne "get_pr_info pour l'obtenir" mais cette dépendance devrait être documentée plus clairement ou gérée automatiquement.

Ligne 69 : Le body racine "Suggestion sur {path}:{line}" est redondant avec le commentaire inline. Cela pourrait créer de la confusion.

Lignes 147-160 : La boucle d'événements for event in agent.stream() affiche directement tous les événements sans filtrage. Cela peut polluer la sortie. Considérez un affichage structuré.

Verdict

❌ Changements requis

Le fichier calculator.py contient plusieurs violations critiques des bonnes pratiques Python :

Gestion d'erreurs dangereuse (except nu)
Effets de bord inappropriés (print dans fonctions)
Non-respect PEP8 (espacement, nomenclature)
Absence de documentation

Les fichiers acte1.py et acte2.py ont un import cassé et quelques problèmes mineurs de style. Corrigez ces points avant merge.

ferhatbe

Suggestion sur calculator.py:14

ferhatbe · 2026-07-28T13:37:48Z

+    r=a+b
+    return r
+
+def traiter_liste(items):


❌ Anti-pattern range(len()) : Itérez directement sur les éléments plutôt que sur les indices. Plus pythonic et lisible.

Suggested change

def traiter_liste(items):

def sum_list(numbers):

"""Calcule la somme d'une liste de nombres.

Args:

numbers: Liste de nombres

Returns:

La somme totale

"""

total = 0

for num in numbers:

total += num

return total

ferhatbe

Suggestion sur calculator.py:7

ferhatbe · 2026-07-28T13:37:48Z

+        resultat = x / y
+        print(resultat)
+        return resultat
+    except:


❌ Gestion d'erreurs dangereuse : except: nu capture toutes les exceptions (y compris KeyboardInterrupt, SystemExit). Spécifiez les exceptions attendues.

Suggested change

except:

except ZeroDivisionError:

return "Erreur : division par zéro"

except TypeError:

return "Erreur : types invalides"

ferhatbe

Suggestion sur calculator.py:3

ferhatbe · 2026-07-28T13:37:48Z

@@ -0,0 +1,16 @@
+# calculator.py — fichier à reviewer (intentionnellement défectueux)
+def diviser(x, y):
+    try:


📝 Documentation manquante : Ajoutez une docstring pour décrire le comportement de la fonction.

Suggested change

try:

def add(a, b):

"""Additionne deux nombres.

Args:

a: Premier nombre

b: Deuxième nombre

Returns:

La somme de a et b

"""

result = a + b

ferhatbe

Suggestion sur calculator.py:4

ferhatbe · 2026-07-28T13:37:48Z

+# calculator.py — fichier à reviewer (intentionnellement défectueux)
+def diviser(x, y):
+    try:
+        resultat = x / y


❌ Effet de bord : print() dans une fonction de calcul viole le principe de responsabilité unique. La fonction doit retourner la valeur sans l'afficher.

Suggested change

resultat = x / y

result = a + b

return result

ferhatbe

Suggestion sur calculator.py:2

ferhatbe · 2026-07-28T13:37:48Z

@@ -0,0 +1,16 @@
+# calculator.py — fichier à reviewer (intentionnellement défectueux)
+def diviser(x, y):


⚠️ PEP8 : Ajoutez une ligne vide après les imports (PEP8 E302).

Suggested change

def diviser(x, y):

def add(a, b):

ferhatbe

Suggestion sur calculator.py:6

ferhatbe · 2026-07-28T13:37:49Z

+    try:
+        resultat = x / y
+        print(resultat)
+        return resultat


📝 Documentation manquante : Ajoutez une docstring expliquant la gestion d'erreur.

Suggested change

return resultat

def divide(a, b):

"""Divise deux nombres avec gestion d'erreur.

Args:

a: Numérateur

b: Dénominateur

Returns:

Le résultat de a / b ou un message d'erreur

"""

ferhatbe

Suggestion sur acte1.py:4

ferhatbe · 2026-07-28T13:37:57Z

+import os
+from dotenv import load_dotenv
+load_dotenv()
+


💡 Suggestion : Utilisez des f-strings pour plus de lisibilité et de performance.

Suggested change

print(f"Résultat: {result}")

ferhatbe

Suggestion sur acte1.py:1

ferhatbe · 2026-07-28T13:37:57Z

@@ -0,0 +1,106 @@
+import os


❌ Import cassé : Le module langchain_classic n'existe pas. Utilisez langchain à la place.

Suggested change

import os

from langchain.agents import AgentExecutor

ferhatbe

Suggestion sur acte1.py:3

ferhatbe · 2026-07-28T13:37:57Z

@@ -0,0 +1,106 @@
+import os
+from dotenv import load_dotenv
+load_dotenv()


⚠️ Nom de variable : Évitez les accents dans les noms de variables (résultat → result). Suivez les conventions Python anglophones.

Suggested change

load_dotenv()

result = 10 + 5

ferhatbe

Suggestion sur acte2.py:4

ferhatbe · 2026-07-28T13:38:04Z

+import os
+from dotenv import load_dotenv
+load_dotenv()
+


💡 Suggestion : Utilisez des f-strings pour plus de lisibilité et de performance.

Suggested change

print(f"Résultat: {result}")

ferhatbe

Suggestion sur acte2.py:3

ferhatbe · 2026-07-28T13:38:04Z

@@ -0,0 +1,162 @@
+import os
+from dotenv import load_dotenv
+load_dotenv()


⚠️ Nom de variable : Évitez les accents dans les noms de variables (résultat → result). Suivez les conventions Python anglophones.

Suggested change

load_dotenv()

result = 20 * 3

ferhatbe

Suggestion sur acte2.py:1

ferhatbe · 2026-07-28T13:38:04Z

@@ -0,0 +1,162 @@
+import os


❌ Import cassé : Le module langchain_classic n'existe pas. Utilisez langchain à la place.

Suggested change

import os

from langchain.agents import initialize_agent

ferhatbe and others added 4 commits July 27, 2026 15:43

Merge pull request #1 from ferhatbe/main

12a25ac

Delete calculator.py

update

16ac42c

Merge branch 'feat/calculator' of https://github.com/ferhatbe/my_pyth…

6b725ae

…on_tp into feat/calculator

mise a jour

e36f523

ferhatbe commented Jul 27, 2026

View reviewed changes

ferhatbe commented Jul 28, 2026

View reviewed changes

mise a jour

64d2c07

ferhatbe commented Jul 28, 2026

View reviewed changes

-def traiter_liste(items):
+def sum_list(numbers):
+    """Calcule la somme d'une liste de nombres.
+    Args:
+        numbers: Liste de nombres
+    Returns:
+        La somme totale
+    """
+    total = 0
+    for num in numbers:
+        total += num
+    return total

-    except:
+    except ZeroDivisionError:
+        return "Erreur : division par zéro"
+    except TypeError:
+        return "Erreur : types invalides"

-    try:
+def add(a, b):
+    """Additionne deux nombres.
+    Args:
+        a: Premier nombre
+        b: Deuxième nombre
+    Returns:
+        La somme de a et b
+    """
+    result = a + b

		@@ -0,0 +1,16 @@
		# calculator.py — fichier à reviewer (intentionnellement défectueux)
		def diviser(x, y):

-        return resultat
+def divide(a, b):
+    """Divise deux nombres avec gestion d'erreur.
+    Args:
+        a: Numérateur
+        b: Dénominateur
+    Returns:
+        Le résultat de a / b ou un message d'erreur
+    """

Conversation

ferhatbe commented Jul 27, 2026

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Revue

Problèmes

calculator.py

acte1.py

Suggestions

Pour calculator.py

Pour acte1.py

Verdict

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

🔍 Revue de code - PR #2

✅ Points positifs

⚠️ Problèmes identifiés

calculator.py (fichier de test)

acte1.py

💡 Suggestions

🎯 Verdict

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

🔍 Revue de code - PR #2

✅ Points positifs

⚠️ Problèmes identifiés

calculator.py (fichier de test)

acte1.py

🎯 Suggestions d'amélioration

📊 Verdict

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Revue

Problèmes

calculator.py

acte1.py

acte2.py

Verdict

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferhatbe left a comment

Choose a reason for hiding this comment

`calculator.py`

`acte1.py`

Pour `calculator.py`

Pour `acte1.py`