fix(matplotlib): corriger le rendu des histogrammes adaptatifs by ElouenGinat · Pull Request #35 · KhiopsML/khisto-python

ElouenGinat · 2026-09-22T12:07:23Z

What

Preserve Khiops frequencies and keep narrow adaptive bins visible.
Reject unsupported histtype="barstacked".
Export hist from the package root and infer its return type from histtype.
Simplify the demo, use root-level imports, and refresh its figures.

Validation

uv run pytest -q
Demo notebook executed end to end
uv run sphinx-build -W --keep-going -b html docs docs/_build/html
Pre-commit hooks

Closes #34

Réutiliser les fréquences Khiops pour les valeurs extrêmes et rendre visibles les classes très étroites par défaut. Refuser également le type barstacked non pris en charge. Closes #34

github-actions · 2026-09-22T12:10:10Z

PR Preview Action v1.8.1
🚀 View preview at https://KhiopsML.github.io/khisto-python/pr-preview/pr-35/
Built to branch `gh-pages` at 2026-09-28 07:50 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Copilot

Copilot review overview

🟡 Changes recommended

The cumulative density implementation, ec edge styling, and stale notebook import remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Cumulative density is incorrect for unequal-width bins · New
Recognize ec alias to preserve explicit edge styling · New

What changed in this PR

Updates the Matplotlib histogram adapter to preserve Khiops frequencies, improve adaptive-bin rendering, and expose hist at the package root.

Changes:

Adds typed histogram returns and rejects unsupported stacked histograms.
Updates plotting behavior, tests, documentation, CI, changelog, and demo metadata.

File	Summary
`tests/plot/test_matplotlib_histogram.py`	Adds histogram behavior and typing tests.
`src/khisto/matplotlib/hist.py`	Implements frequency-backed plotting and return handling; cumulative density behavior and `ec` styling require changes.
`src/khisto/__init__.py`	Exports `hist` at the package root.
`sandbox/khisto_demo.ipynb`	Updates demo metadata; retains a stale `_hist` import that causes an import error.
`pyproject.toml`	Adds the Python 3.10 typing dependency.
`docs/index.rst`	Updates usage examples.
`docs/conf.py`	Configures Matplotlib API documentation.
`docs/api_comparison.md`	Documents unsupported histogram options.
`CHANGELOG.md`	Records the histogram changes.
`.github/workflows/ci.yaml`	Adds optional-dependency installation coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…ct for unequal-width bins' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

marcboulle

J'ai bien aimé la simplification de la démo ;)

Pour le reste, je n'ai fait que survolé le code rapidement. Je ne comprend pas tous les détails et le pourquoi des évolutions. C'est suite aux retours utilisateurs? ou à des tests approfondis?

J'ai quelques remarques et surtout des questions de détail.

popescu-v · 2026-09-23T14:49:04Z

+
+      - name: Test base API without optional dependencies
+        run: |
+          uv run --isolated --no-project --with . python - <<'PY'


I would put this Python snippet in a separate script for easier maintenance.

popescu-v · 2026-09-23T17:16:35Z

+    elif isinstance(patches, list):
+        histogram_patches = [patch for patch in patches if isinstance(patch, Polygon)]
+        if len(histogram_patches) != len(patches):
+            raise TypeError("Matplotlib returned unexpected histogram patches.")


I'd be more specific on the error message:

unexpected_patches = [ patch for patch in patches if not isinstance(path, Polygon) ] ... raise TypeError( f"Matplotlib returned unexpected histogram patches: {len(unexpected_patches)}" f" of them are of types {', '.join(set([type(patch) for patch in unexpected_patches])}." )

the patches are all of type Polygon or all of type BarContainer. I made the message clearer but i didn't listed all the patches in the message.

popescu-v · 2026-09-23T17:18:11Z

+        if len(histogram_patches) != len(patches):
+            raise TypeError("Matplotlib returned unexpected histogram patches.")
+    else:
+        raise TypeError("Matplotlib returned unexpected histogram patches.")


I'd be more specific in the message here as well, specifying the type(patches) therein.

popescu-v · 2026-09-23T17:23:33Z

+    else:
+        raise TypeError("Matplotlib returned unexpected histogram patches.")
+
+    if histtype == "bar" and not {"edgecolor", "ec"} & kwargs.keys():


For more readability, I'd write this if as:

if histtype == "bar" and not {"edgecolor", "ec"}.intersection(kwargs.keys()):

popescu-v · 2026-09-23T17:28:16Z

@@ -6,11 +6,20 @@

 from __future__ import annotations


Do we need to treat typing hints as strings here? Their eager evaluation as objects is not adequate here?

i removed it

Un pyproject.toml étranger présent dans site-packages faisait planter l'import de khisto (fichier ouvert en mode texte). Le pyproject.toml du dépôt n'est désormais lu que si le paquet n'est pas installé.

Commente l'intention (rendre visibles les classes plus fines qu'un pixel) et précise le message d'erreur sur les patches inattendus.

marcboulle

Cela me parait bien, en tout cas d'un point de vue fonctionnel.

Un point de détail sur le CHANGELOG, à améliorer éventuellement.

Pour les tests unitaires et de non régression, je fais confiance au mainteneur.

fix(matplotlib): corriger le rendu des histogrammes adaptatifs

7e4549f

Réutiliser les fréquences Khiops pour les valeurs extrêmes et rendre visibles les classes très étroites par défaut. Refuser également le type barstacked non pris en charge. Closes #34

ElouenGinat linked an issue Sep 22, 2026 that may be closed by this pull request

Fix invisible narrow bins and incorrect extreme-value counts #34

Open

feat(api): exposer hist au niveau racine

13830b5

ElouenGinat added 7 commits September 22, 2026 14:10

docs(demo): simplifier le guide et actualiser les figures

f7c14d4

feat(matplotlib): préciser les types de retour de hist

63afdbd

docs(api): utiliser les imports au niveau racine

731d2d4

ci: tester l'installation sans matplotlib

aa94a3b

fix(tests): prendre en charge assert_type sous Python 3.10

2355d70

docs(sphinx): masquer les surcharges dans les signatures

c40fc12

docs(sphinx): restaurer les types dans les signatures

fbdb9d3

ElouenGinat requested review from marcboulle and popescu-v and a lite review from Copilot and removed request for marcboulle September 22, 2026 14:09

Copilot started reviewing on behalf of ElouenGinat September 22, 2026 14:09 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread src/khisto/matplotlib/hist.py Outdated

Comment thread src/khisto/matplotlib/hist.py Outdated

ElouenGinat and others added 2 commits September 22, 2026 16:20

Potential fix for pull request finding 'Cumulative density is incorre…

89f47ab

…ct for unequal-width bins' Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

fix(matplotlib): préserver l'alias ec de la couleur de bord

a9e7844

marcboulle requested changes Sep 22, 2026

View reviewed changes

Comment thread docs/demo.ipynb Outdated

Comment thread docs/demo.ipynb

Comment thread src/khisto/matplotlib/hist.py Outdated

Comment thread src/khisto/matplotlib/hist.py

fix(matplotlib): transmettre les données sans conversion en listes

6f687db

popescu-v reviewed Sep 23, 2026

View reviewed changes

ElouenGinat added 3 commits September 24, 2026 16:26

fix(version): lire la version installée avant le pyproject.toml

df5fdb6

Un pyproject.toml étranger présent dans site-packages faisait planter l'import de khisto (fichier ouvert en mode texte). Le pyproject.toml du dépôt n'est désormais lu que si le paquet n'est pas installé.

refactor(matplotlib): clarifier le forçage de la couleur de bord

54cd2f2

Commente l'intention (rendre visibles les classes plus fines qu'un pixel) et précise le message d'erreur sur les patches inattendus.

test(matplotlib): supprimer l'import inutile de __future__.annotations

9424891

ElouenGinat added 2 commits September 24, 2026 16:26

ci: extraire le test d'installation de base dans un script

9c23f18

fix(sandbox): supprimer l'import obsolète de _hist

95450e0

ElouenGinat requested review from marcboulle and popescu-v September 25, 2026 08:57

marcboulle approved these changes Sep 25, 2026

View reviewed changes

Comment thread CHANGELOG.md

doc: better changelog

15b6978

Conversation

ElouenGinat commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Validation

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2026-09-28 07:50 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

marcboulle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

popescu-v Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

marcboulle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ElouenGinat commented Sep 22, 2026 •

edited

Loading

github-actions Bot commented Sep 22, 2026 •

edited

Loading

Built to branch `gh-pages` at 2026-09-28 07:50 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

popescu-v Sep 23, 2026 •

edited

Loading