Skip to content

Add CoclusteringCell part_indexes attribute for getting per-cell part indexes#574

Merged
popescu-v merged 1 commit intomainfrom
569-need-to-get-part-indexes-for-coclustering-cells-in-khiopscorecoclustering_resultscoclusteringcell
Apr 21, 2026
Merged

Add CoclusteringCell part_indexes attribute for getting per-cell part indexes#574
popescu-v merged 1 commit intomainfrom
569-need-to-get-part-indexes-for-coclustering-cells-in-khiopscorecoclustering_resultscoclusteringcell

Conversation

@popescu-v
Copy link
Copy Markdown
Collaborator

@popescu-v popescu-v commented Apr 20, 2026

The CoclusteringCell.part_indexes attribute is initialized from the cellPartIndexes JSON attribute directly.

closes #569.


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

@popescu-v popescu-v self-assigned this Apr 20, 2026
@popescu-v popescu-v requested review from marcboulle and tramora April 20, 2026 16:57
@popescu-v popescu-v force-pushed the 569-need-to-get-part-indexes-for-coclustering-cells-in-khiopscorecoclustering_resultscoclusteringcell branch 2 times, most recently from 903a878 to 80041a4 Compare April 20, 2026 17:15
Copy link
Copy Markdown

@marcboulle marcboulle left a comment

Choose a reason for hiding this comment

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

Je ne comprend pas les propositions, qui ne répondent pas simplement au besoin exprimé.
Le fichier json contient des informations selon un format le plus simple possible à exploiter, et les classes python devraient juste être un wrapper minimaliste, permettant de retrouver simplement les information du json, avec éventuellement des helpers.

En l'occurrence, il me semble que le seul ajout a effectuer est d'ajouter l'accès à cellPartIndexespar cellules, via le nouvel attribut part_indexes, l'attribut existant parts ayant un rôle de helper pour accéderr directement aux objets concernés.

class CoclusteringCell:
    """A coclustering cell

    .. note::
        This class has only a no-parameter constructor initializing an instance with the
        default values.

    Attributes
    ----------
    part_indexes : list of int
        Part indexes for each dimension of the coclustering.
    parts : list of `CoclusteringDimensionPart`
        Parts for each dimension of the coclustering, associated with the corresponding indexes. 
    frequency : int
        Frequency of this cell.
    """

@popescu-v
Copy link
Copy Markdown
Collaborator Author

popescu-v commented Apr 21, 2026

Je ne comprend pas les propositions, qui ne répondent pas simplement au besoin exprimé. Le fichier json contient des informations selon un format le plus simple possible à exploiter, et les classes python devraient juste être un wrapper minimaliste, permettant de retrouver simplement les information du json, avec éventuellement des helpers.

En l'occurrence, il me semble que le seul ajout a effectuer est d'ajouter l'accès à cellPartIndexespar cellules, via le nouvel attribut part_indexes, l'attribut existant parts ayant un rôle de helper pour accéderr directement aux objets concernés.

class CoclusteringCell:
    """A coclustering cell

    .. note::
        This class has only a no-parameter constructor initializing an instance with the
        default values.

    Attributes
    ----------
    part_indexes : list of int
        Part indexes for each dimension of the coclustering.
    parts : list of `CoclusteringDimensionPart`
        Parts for each dimension of the coclustering, associated with the corresponding indexes. 
    frequency : int
        Frequency of this cell.
    """

Ok, it seems simpler indeed to just add a CoclusteringCell.part_indexes attribute and initialize it in CoclusteringReport.__init__ when the CoclusteringCell instances are created and initialized.

@popescu-v popescu-v force-pushed the 569-need-to-get-part-indexes-for-coclustering-cells-in-khiopscorecoclustering_resultscoclusteringcell branch from 80041a4 to e5c261b Compare April 21, 2026 09:51
@popescu-v popescu-v requested a review from marcboulle April 21, 2026 09:52
@popescu-v popescu-v changed the title Add methods for getting per-cell part indexes Add CoclusteringCell part_indexes attribute for getting per-cell part indexes Apr 21, 2026
Copy link
Copy Markdown

@marcboulle marcboulle left a comment

Choose a reason for hiding this comment

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

LGTM

@tramora
Copy link
Copy Markdown
Collaborator

tramora commented Apr 21, 2026

Looks good for me but is it possible to add the case in the automated test in test_core.py : test_coclustering_results_accessors

@popescu-v
Copy link
Copy Markdown
Collaborator Author

Looks good for me but is it possible to add the case in the automated test in test_core.py : test_coclustering_results_accessors

In the latest update there are no more accessors related to the scope of the PR. It's just an attribute that is being initialized directly from the JSON file.
The correct initialization of this attribute is indirectly tested via the CoclusteringReport.to_dict method which uses the attribute and is called in the CoclusteringResults.to_dict method, which is called in the CoclusteringResults.write_khiops_json_file method, which is tested in the KhiopsCoreIOTests._assert_coclustering_report_is_written_to_sorted_json_file method, which is called in the KhiopsCoreIOTests.test_coclustering_results testing method.

@popescu-v popescu-v merged commit 16936e1 into main Apr 21, 2026
18 checks passed
@popescu-v popescu-v deleted the 569-need-to-get-part-indexes-for-coclustering-cells-in-khiopscorecoclustering_resultscoclusteringcell branch April 21, 2026 11:53
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.

Need to get part indexes for coclustering cells in khiops.core.coclustering_results.CoclusteringCell

3 participants