Skip to content

Rename variable_part_dimensions to inner_variable_dimensions in Coclustering results#568

Open
tramora wants to merge 1 commit intomainfrom
552-refactor-coclustering-inner-variables
Open

Rename variable_part_dimensions to inner_variable_dimensions in Coclustering results#568
tramora wants to merge 1 commit intomainfrom
552-refactor-coclustering-inner-variables

Conversation

@tramora
Copy link
Collaborator

@tramora tramora commented Mar 25, 2026

  • test_coclustering_results_simple_initializations remains unchanged
  • the CoclusteringDimension docstring is amended to give more details on "Instance * Variables Coclustering"

Fixes #552

The docstring amendment in CoclusteringDimension for inner_variable_dimensions is far from perfect and may need correction


TODO Before Asking for a Review

  • Rebase your branch to the latest version of main (or main-v10)
  • Make sure all CI workflows are green
  • When adding a public feature/fix: Update the Unreleased section of CHANGELOG.md (no date)
  • Self-Review: Review "Files Changed" tab and fix any problems you find
  • API Docs (only if there are changes in docstrings, rst files or samples):
    • Check the docs build without warning: see the log of the API Docs workflow
    • Check that your changes render well in HTML: download the API Docs artifact and open index.html
    • If there are any problems it is faster to iterate by building locally the API Docs

@tramora tramora requested a review from popescu-v March 25, 2026 15:54
CoclusteringDimension
|- parts -> list of CoclusteringDimensionPart
|- variable_part_dimensions -> list of CoclusteringDimension
|- inner_variable_dimensions -> list of CoclusteringDimension
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shift all arrows one space to the right to keep them aligned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

inner_variable_dimensions : list of `CoclusteringDimension`
Variable part instance-variable coclustering dimensions. ``None`` for
variable-variable clustering.
This list is filled only in an "Instance * Variables coclustering" context.
Copy link
Collaborator

@popescu-v popescu-v Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would replace all this paragraph with (to check with @marcboulle):

"""
Contains the dimensions of the variable parts of each variable as computed via the individual * variable coclustering algorithm.
"""

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the reformulation. I'm waiting for @marcboulle suggestion but it the meantime I put yours.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marc validates but wants to add a precision. I put this later version.

Copy link
Collaborator

@popescu-v popescu-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two docstring changes (see the comments).

@tramora tramora force-pushed the 552-refactor-coclustering-inner-variables branch from 5d917d5 to ae8ee70 Compare March 25, 2026 16:18
…oclustering results

- `test_coclustering_results_simple_initializations` remains unchanged
- the `CoclusteringDimension` docstring is amended to give more details on "Instances x Variables Coclustering"
@tramora tramora force-pushed the 552-refactor-coclustering-inner-variables branch from ae8ee70 to 6ddf11a Compare March 25, 2026 16:36
@tramora tramora requested a review from popescu-v March 25, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make the Instance-Variable Coclustering Inner Variables API More Easily Discoverable

2 participants