Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pairs documentation #311

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

folmos-at-orange
Copy link
Member

@folmos-at-orange folmos-at-orange commented Dec 12, 2024

Put your message here


TODO Before Asking for a Review

  • Rebase your branch to the latest version of dev (or main for release PRs)
  • 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

@folmos-at-orange folmos-at-orange linked an issue Dec 12, 2024 that may be closed by this pull request
@folmos-at-orange folmos-at-orange force-pushed the 243-improve-specific_pairs-documentation branch from 1ba74d2 to bf157b4 Compare December 12, 2024 15:37
@folmos-at-orange folmos-at-orange self-assigned this Dec 12, 2024
which jointly are more informative that its univariate components may be taken
into account in the classifier.
Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/partition feature pair/partition of the domain of a feature pair/

Copy link
Member Author

@folmos-at-orange folmos-at-orange Dec 13, 2024

Choose a reason for hiding this comment

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

I eliminated "domain" because is too verbose (two "of" one after another). That the partition is in the domain is implied by the the 2D grid part, there is no ambiguity. I'll add the missing "the" nonetheless.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
distribution is well approximated by a constant histogram. Only pairs that are
jointly more informative than its marginals may be taken into account in the
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/than its marginals/than their marginals/

(the pairs' marginals)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

containing it (within the maximum limit ``n_pairs``). These pairs have top
priority: they are constructed first.
all_possible_pairs : bool, default ``True``
If ``True`` tries to create all possible pairs within the limit ``n_pairs``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/within the limit/within the maximum limit/

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

which jointly are more informative that its univariate components may be taken
into account in the regressor.
Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/partition feature pair/partition of the domain of a feature pair/

Copy link
Member Author

Choose a reason for hiding this comment

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

See above.

Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
distribution is well approximated by a constant histogram. Only pairs that are
jointly more informative than its marginals may be taken into account in the
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/than its marginals/than their marginals/

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

containing it (within the maximum limit ``n_pairs``). These pairs have top
priority: they are constructed first.
all_possible_pairs : bool, default ``True``
If ``True`` tries to create all possible pairs within the limit ``n_pairs``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/within the limit/within the maximum limit/

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

partition of the domain of a pair of features in which is optimized in a way
that the cells are the purest possible with respect to the target.
Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/partition feature pair/partition of the domain of a feature pair/

Copy link
Member Author

@folmos-at-orange folmos-at-orange Dec 13, 2024

Choose a reason for hiding this comment

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

See above.

Maximum number of pair features to construct. These features are a 2D grid
partition feature pair. The grid is optimized such that in each cell the target
distribution is well approximated by a constant histogram. Only pairs that are
jointly more informative than its marginals may be taken into account in the
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/than its marginals/than their marginals/

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

containing it (within the maximum limit ``n_pairs``). These pairs have top
priority: they are constructed first.
all_possible_pairs : bool, default ``True``
If ``True`` tries to create all possible pairs within the limit ``n_pairs``.
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/within the limit/within the maximum limit/

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

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.

There are few rephrasings to consider (see the comments).

Copy link
Collaborator

@nairbenrekia nairbenrekia left a comment

Choose a reason for hiding this comment

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

Looks all good to me. I have no comments.

@folmos-at-orange folmos-at-orange force-pushed the 243-improve-specific_pairs-documentation branch from bf157b4 to 39b3021 Compare December 16, 2024 10:01
- Remove "source_repository" setting
 - It generated a link to a 404 page in Github
- Change the Github icon to khiops-python
@folmos-at-orange folmos-at-orange force-pushed the 243-improve-specific_pairs-documentation branch from 14ba038 to e947fea Compare December 19, 2024 18:07
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.

Improve specific_pairs documentation
3 participants