Fitting refactor : supplementary and categorical input - #223
Open
ouakouak wants to merge 13 commits into
Open
Conversation
This was referenced May 25, 2026
MaxHalford
requested changes
Jun 6, 2026
Owner
|
@ouakouak apologies for the delay reviewing! |
Author
|
No worries! I have an implementation that suits the one I need for my job at work. So no pressure on it, let's just build a consistent tool! |
Owner
|
@ouakouak I'm a bit lost with your PRs, so please remind me what needs reviewing and merging! |
Author
|
Ok so this branch needs to be merged (if you are done with rewiewing it). It was to require users to input categorical columns as a list in order to be an attribute of the class. It also enables to input supplementary variables in the fitting method (consistent with PCA's implementation). |
Author
Owner
|
Ok clear, thanks. Could you rebase on main and figure out the broken tests? |
4 tasks
MaxHalford
force-pushed
the
fit-coords-and-supp
branch
from
June 17, 2026 19:10
23cb973 to
febef6b
Compare
MaxHalford
added a commit
to ouakouak/prince
that referenced
this pull request
Jun 17, 2026
…es_ to Pagès 2004 Rebuilds the PR on top of master and applies the following changes: - Default `categorical_columns=None` falls back to dtype-based auto-detection so existing call sites (`FAMD().fit(df)`) keep working. - Restore `rescale_with_mean=True` default — silent math change otherwise. - Drop the new `active_*` / `supplementary_*` public attributes. `num_cols_`, `cat_cols_`, `num_scaler_`, `one_hot_columns_`, `categories_` keep their master semantics; active vs supplementary is derivable (e.g., `num_scaler_.feature_names_in_` for the active numerical set). - Fix two latent typos in `row_coordinates` that swapped numerical vs categorical column references (only present in the renamed code paths). These weren't caught because the PR's `fit` raised before any tests reached `row_coordinates`; with auto-detection restored, `test_row_coords` covers the path. - Cast numerical columns to float at fit/transform — pandas refuses to write a float ndarray into an int dtype with `X[:] = ...`, which would surface when users pass int columns as numerical via `categorical_columns=[...]`. - `column_coordinates_` now stores genuine PCA coordinates per Pagès (2004, §5.1, p. 98–99): G_s(k) = r(k, F_s) for numerical, G_s(k_q) = √(p/λ) · F_s for modalities. The eta-squared aggregation moves into `column_correlations` for the categorical part, using η²(q, s) = Σ G_s(k_q)² (the cross-factors cancel — derivation in the issue MaxHalford#215 thread). - `column_contributions_` becomes `f²/λ` (modality-level), consistent with the redefined coordinates. - Tests adapted to the new coord/contrib semantics: variable-level FactoMineR-style `var$coord`/`var$contrib` are reconstructed by squaring + per-variable aggregation. New tests cover explicit `categorical_columns` override and the `supplementary_columns=` fit path. Refs: MaxHalford#215, MaxHalford#223, husson/FactoMineR#37, Pagès (2004) Analyse factorielle de données mixtes.
Owner
|
@ouakouak I allowed myself to clean up the code to follow some of Prince's style idioms; I hope that's ok. Here's a summary from Claude:
|
MaxHalford
self-requested a review
June 17, 2026 20:07
MaxHalford
approved these changes
Jun 17, 2026
MaxHalford
added a commit
to ouakouak/prince
that referenced
this pull request
Jun 17, 2026
…es_ to Pagès 2004 Rebuilds the PR on top of master and applies the following changes: - Default `categorical_columns=None` falls back to dtype-based auto-detection so existing call sites (`FAMD().fit(df)`) keep working. - Restore `rescale_with_mean=True` default — silent math change otherwise. - Drop the new `active_*` / `supplementary_*` public attributes. `num_cols_`, `cat_cols_`, `num_scaler_`, `one_hot_columns_`, `categories_` keep their master semantics; active vs supplementary is derivable (e.g., `num_scaler_.feature_names_in_` for the active numerical set). - Fix two latent typos in `row_coordinates` that swapped numerical vs categorical column references (only present in the renamed code paths). These weren't caught because the PR's `fit` raised before any tests reached `row_coordinates`; with auto-detection restored, `test_row_coords` covers the path. - Cast numerical columns to float at fit/transform — pandas refuses to write a float ndarray into an int dtype with `X[:] = ...`, which would surface when users pass int columns as numerical via `categorical_columns=[...]`. - `column_coordinates_` now stores genuine PCA coordinates per Pagès (2004, §5.1, p. 98–99): G_s(k) = r(k, F_s) for numerical, G_s(k_q) = √(p/λ) · F_s for modalities. The eta-squared aggregation moves into `column_correlations` for the categorical part, using η²(q, s) = Σ G_s(k_q)² (the cross-factors cancel — derivation in the issue MaxHalford#215 thread). - `column_contributions_` becomes `f²/λ` (modality-level), consistent with the redefined coordinates. - Tests adapted to the new coord/contrib semantics: variable-level FactoMineR-style `var$coord`/`var$contrib` are reconstructed by squaring + per-variable aggregation. New tests cover explicit `categorical_columns` override and the `supplementary_columns=` fit path. Refs: MaxHalford#215, MaxHalford#223, husson/FactoMineR#37, Pagès (2004) Analyse factorielle de données mixtes.
MaxHalford
force-pushed
the
fit-coords-and-supp
branch
from
June 17, 2026 20:50
9f4cf57 to
b952639
Compare
…tive and supplementary. Force to input categorical columns.
…es_ to Pagès 2004 Rebuilds the PR on top of master and applies the following changes: - Default `categorical_columns=None` falls back to dtype-based auto-detection so existing call sites (`FAMD().fit(df)`) keep working. - Restore `rescale_with_mean=True` default — silent math change otherwise. - Drop the new `active_*` / `supplementary_*` public attributes. `num_cols_`, `cat_cols_`, `num_scaler_`, `one_hot_columns_`, `categories_` keep their master semantics; active vs supplementary is derivable (e.g., `num_scaler_.feature_names_in_` for the active numerical set). - Fix two latent typos in `row_coordinates` that swapped numerical vs categorical column references (only present in the renamed code paths). These weren't caught because the PR's `fit` raised before any tests reached `row_coordinates`; with auto-detection restored, `test_row_coords` covers the path. - Cast numerical columns to float at fit/transform — pandas refuses to write a float ndarray into an int dtype with `X[:] = ...`, which would surface when users pass int columns as numerical via `categorical_columns=[...]`. - `column_coordinates_` now stores genuine PCA coordinates per Pagès (2004, §5.1, p. 98–99): G_s(k) = r(k, F_s) for numerical, G_s(k_q) = √(p/λ) · F_s for modalities. The eta-squared aggregation moves into `column_correlations` for the categorical part, using η²(q, s) = Σ G_s(k_q)² (the cross-factors cancel — derivation in the issue MaxHalford#215 thread). - `column_contributions_` becomes `f²/λ` (modality-level), consistent with the redefined coordinates. - Tests adapted to the new coord/contrib semantics: variable-level FactoMineR-style `var$coord`/`var$contrib` are reconstructed by squaring + per-variable aggregation. New tests cover explicit `categorical_columns` override and the `supplementary_columns=` fit path. Refs: MaxHalford#215, MaxHalford#223, husson/FactoMineR#37, Pagès (2004) Analyse factorielle de données mixtes.
…ordinates_
- Replace fragile `m.startswith(f"{col}_")` modality lookup with
`_modalities_for(col)` derived from the stored `categories_` dict, which
avoids ambiguity when one categorical column name is a prefix of another.
- Move the `_mca_code` helper to module scope (it was a method but uses no
state) and rename for clarity vs the now-removed `_preprocess_categorical`.
- Document the public attribute set (`num_cols_`, `cat_cols_`, `num_scaler_`,
`one_hot_columns_`, `categories_`) in the class docstring, including how to
derive active vs supplementary subsets from existing attrs.
- Trim verbose inline comments that duplicated docstring content.
- Fix the "Figure 2 — Relationship square" cell in `docs/content/famd.ipynb`:
the cell hard-coded `["quanti"] * 3 + ["quali"] * 3` over the variable-level
`column_coordinates_`. With the new modality-level semantics that shape no
longer applies. Now reconstructs FactoMineR-style `var$coord` (r² for
numerical, η² aggregated for categorical) via `column_correlations`, which
is what the relationship square actually visualizes.
…e_coordinates_ (per-variable) Refines the API so that the per-column granularity matches MCA's convention (one row per preprocessed column = one row per modality for cat vars) while still providing the FactoMineR-style variable-level view as a first-class attribute. - `column_coordinates_` = genuine PCA coordinates of the preprocessed columns: one row per active numerical variable (signed correlation = FactoMineR's `quanti.var$coord`) and one row per modality (G_s(k_q) per Pagès 2004 §5.1, i.e. the PCA coord on the MCA-coded indicator). Matches MCA's per-modality convention so the two classes are consistent. - `column_contributions_` = `column_coordinates_² / λ`, one row per preprocessed column. Summing the modalities of a categorical recovers the variable-level contribution. - `variable_coordinates_` *(new)* = FactoMineR's `var$coord`: r² for numerical and η² for categorical, one row per original variable. Convenient for the relationship-square plot. - `variable_contributions_` *(new)* = `variable_coordinates_ / λ`, matching FactoMineR's `var$contrib`. Tests now cover all four FactoMineR per-type outputs directly: - `column_coordinates_.loc[num] = quanti.var$coord` (up to sign). - `column_contributions_.loc[num] * 100 = quanti.var$contrib`. - `column_coordinates_.loc[mod] · √(λ/p) = quali.var$coord` (Pagès relation). - `column_contributions_.loc[mod] * 100 = quali.var$contrib`. - `variable_coordinates_ = var$coord` and `variable_contributions_ * 100 = var$contrib`. The Wikipedia notebook cell now uses `variable_coordinates_` directly (`cc = wiki_famd.variable_coordinates_.iloc[:, :2]`), no manual aggregation.
Pre-commit was running ruff via its own `ruff-pre-commit` hook pinned at v0.8.0, while `uv run ruff` locally resolved to whatever satisfied `>=0.8.8` in pyproject (currently 0.15.4 from uv.lock). The two ruffs drifted in formatting defaults — local lint passed, CI failed. Switch the ruff hooks to `repo: local`, `language: system`, invoked via `uv run --frozen ruff …`. Single source of truth: uv.lock. Bumping ruff is done with `uv lock --upgrade-package ruff`, no second place to keep in sync. CI needs `astral-sh/setup-uv` before `prek-action` so `uv run` is on PATH.
Apply the same approach as MaxHalford#239 to FAMD's one in-house sum-of-squares reduction: replace `(M**2).sum(axis=0)` over the modality coordinates with `np.einsum("ij,ij->j", M, M)`, which computes the per-component column sum of squares without materialising the M**2 temporary (mirrors CA's total-inertia `einsum("ij,ij->", S, S)`). FAMD delegates its n-sized weighted reductions to PCA.fit, which MaxHalford#239 already covers, so this is the only FAMD-local kernel of that shape. Impact is marginal — the modality count is small — but it keeps the idiom consistent across the codebase. Math is identical; FAMD tests pass and ruff is clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the static-type errors in famd.py without changing behaviour: - Align the override signatures with PCA so they are Liskov-compatible: `inverse_transform(self, X, as_array=False)`, `row_standard_coordinates` gains a default, and `column_cosine_similarities_` becomes a `@property` (matching the base property it shadows). All three are NotImplementedError stubs, so this is signature-only. - `fit` now accepts `sample_weight`/`column_weight` to match PCA's signature but raises NotImplementedError if either is passed — FAMD's preprocessing is computed unweighted, so silently honouring them would be wrong. - Replace `Series.astype(str).unique()` with `set(Series.astype(str))` in the unknown-category check (same result, and the static type stays a Series). - Annotate the two genuine sklearn-stubs gaps (`check_array(dtype=[...])` and `StandardScaler.feature_names_in_`) with targeted pyright suppressions. Pyright: 0 errors. `ty` (CI), ruff and the full suite (669 passed) unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MaxHalford
force-pushed
the
fit-coords-and-supp
branch
from
June 19, 2026 13:54
9d16ca5 to
34958fa
Compare
The lint job ran ruff via `uv run --frozen --extra dev`, but the `dev` extra pulls in rpy2, which can't build on the runner without R, so ruff never ran. Add a lightweight `lint` extra (just ruff) that `dev` self-references, and point the pre-commit ruff/ruff-format hooks at it. Also apply the pending ruff autofixes to the docs notebooks (import sorting, required `from __future__ import annotations`, quote/format normalization, and an unused pandas import). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This first PR enables users to pass categorical features as attributes of the class, and also introduces support for supplementary variables in the
fitmethod.At the moment, users are essentially required to explicitly specify categorical features, as no automatic type detection has been implemented yet. We could consider adding such a feature later if needed.
Regarding supplementary variables, I have only added the necessary preprocessing step and passed them as arguments to
super().fit.Apart from that, the rest of the implementation remains unchanged, aside from some renaming of attributes. All previously implemented parts (except for the initial preprocessing before the call to
super().fit) have simply been adapted to the new distinction between active attributes.For now, supplementary variables are fitted but not fully integrated into the downstream computations.
Also added a small description to the class but can be modified or improved if needed.