Skip to content

312 accept types in internal csv reading function#314

Merged
folmos-at-orange merged 2 commits intodevfrom
312-accept-types-in-internal-csv-reading-function
Jan 8, 2025
Merged

312 accept types in internal csv reading function#314
folmos-at-orange merged 2 commits intodevfrom
312-accept-types-in-internal-csv-reading-function

Conversation

@folmos-at-orange
Copy link
Copy Markdown
Member

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

This will enable an easier implementation of #74 and #74 in #306.

commit 149fb30b1ed6f7e6562d1405d38a02388c6a56dc (HEAD -> 312-accept-types-in-internal-csv-reading-function, origin/312-accept-types-in-internal-csv-reading-function)
Author: Felipe Olmos <92923444+folmos-at-orange@users.noreply.github.com>
Date:   Tue Dec 17 17:01:40 2024 +0100

    Control the types on sklearn internal read table

    This is done only for KhiopsClassifier and KhiopsRegressor. It is
    critical for KhiopsClassifier as it accepts many target types.

    For KhiopsEncoder and KhiopsCoclustering is less critical and for the
    first one it is very complex.

commit 69dce771f41cf2c88f2cd8f757752e9f09f43854
Author: Felipe Olmos <92923444+folmos-at-orange@users.noreply.github.com>
Date:   Tue Dec 17 17:05:22 2024 +0100

    Fix a pylint nit

commit 9fafd2acd433330a9caf9712ee5b67e986d4928c
Author: Felipe Olmos <92923444+folmos-at-orange@users.noreply.github.com>
Date:   Tue Dec 17 17:13:16 2024 +0100

    Rename transform_pairs parameter to transform_type_pairs

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 17, 2024 that may be closed by this pull request
@folmos-at-orange folmos-at-orange force-pushed the 312-accept-types-in-internal-csv-reading-function branch 3 times, most recently from c579618 to b21e13c Compare December 18, 2024 08:48
@folmos-at-orange folmos-at-orange self-assigned this Dec 18, 2024
@folmos-at-orange folmos-at-orange force-pushed the 312-accept-types-in-internal-csv-reading-function branch from b21e13c to bd99eaa Compare December 19, 2024 18:01
Comment thread khiops/sklearn/estimators.py
Comment thread khiops/sklearn/estimators.py
Comment thread khiops/sklearn/estimators.py Outdated
Comment thread khiops/sklearn/estimators.py Outdated
Comment thread khiops/utils/dataset.py
Comment thread khiops/utils/dataset.py
Comment thread tests/test_sklearn_output_types.py
@folmos-at-orange folmos-at-orange force-pushed the 312-accept-types-in-internal-csv-reading-function branch 2 times, most recently from 632bdf1 to 2bfa5d5 Compare January 7, 2025 09:11
Comment thread khiops/sklearn/dataset.py Outdated
@folmos-at-orange folmos-at-orange force-pushed the 312-accept-types-in-internal-csv-reading-function branch 2 times, most recently from 6d672b5 to 0527279 Compare January 8, 2025 08:55
We do this only for KhiopsClassifier and KhiopsRegressor: It is critical
for KhiopsClassifier as it accepts many target types and it is trivial
in the case of KhiopsRegressor.

For KhiopsEncoder and KhiopsCoclustering is less critical and for the
first one it is very complex. We left them as TODO's.

Additionaly, we now also check in the "output type" tests that the
result of predict is correct. Before we only checked only that the
classes_ attribute was ok. This is to further ensure correctness.
@folmos-at-orange folmos-at-orange force-pushed the 312-accept-types-in-internal-csv-reading-function branch from 0527279 to 3bf819f Compare January 8, 2025 11:52
@folmos-at-orange folmos-at-orange merged commit 1012a6f into dev Jan 8, 2025
@folmos-at-orange folmos-at-orange deleted the 312-accept-types-in-internal-csv-reading-function branch January 8, 2025 12:11
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.

Accept types in internal CSV reading function

2 participants