Conversation
0705f18 to
7917714
Compare
7917714 to
c3df671
Compare
folmos-at-orange
requested changes
Mar 13, 2025
Member
folmos-at-orange
left a comment
There was a problem hiding this comment.
Lots of small stuff. A few not so small.
tests/resources/scenario_generation/api/train_predictor/ref/SpliceJunction._kh
Show resolved
Hide resolved
c3df671 to
524f8be
Compare
524f8be to
fb040a1
Compare
fb040a1 to
59dd770
Compare
folmos-at-orange
requested changes
Mar 14, 2025
Member
folmos-at-orange
left a comment
There was a problem hiding this comment.
Found with the typos tool:
error: `intepretation` should be `interpretation`
--> khiops/samples/samples.py:788:15
|
788 | """Builds intepretation model for existing predictor
| ^^^^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/samples/samples.py:815:17
|
815 | print(f"The intepretation model is '{interpretor_file_path}'")
| ^^^^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/samples/samples.ipynb:835:13
|
835 | "Builds intepretation model for existing predictor\n\n It calls `~.api.train_predictor` and `~.api.interpret_predictor` only with\n their mandatory parameters.\n \n"
| ^^^^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/samples/samples.ipynb:866:19
|
866 | "print(f\"The intepretation model is '{interpretor_file_path}'\")"
| ^^^^^^^^^^^^^
|
error: `Intepretation` should be `Interpretation`
--> khiops/core/internals/tasks/interpret_predictor.py:36:12
|
36 | // Intepretation settings
| ^^^^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/core/internals/tasks/interpret_predictor.py:50:18
|
50 | // Build intepretation dictionary
| ^^^^^^^^^^^^^
|
error: `intepret` should be `interpret`
--> khiops/core/api.py:318:35
|
318 | "'khiops.core.api.intepret_predictor' function is not supported "
| ^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/core/api.py:825:19
|
825 | r"""Builds an intepretation dictionary from a predictor
| ^^^^^^^^^^^^^
|
error: `intepretor` should be `interpreter`
--> khiops/core/api.py:833:5
|
833 | intepretor_file_path : str
| ^^^^^^^^^^
|
error: `intepretor` should be `interpreter`
--> khiops/core/api.py:834:21
|
834 | Path to the intepretor dictionary file.
| ^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/core/api.py:836:70
|
836 | Maximum number of variable importances to be selected in the intepretation
| ^^^^^^^^^^^^^
|
error: `intepretation` should be `interpretation`
--> khiops/core/api.py:845:9
|
845 | intepretation model. Min length: 0. Max length: the total number of variables
| ^^^^^^^^^^^^^
|
error: `intialization` should be `initialization`
--> tests/test_khiops_integrations.py:202:51
|
202 | # Get KHIOPS_API_MODE as set after runner intialization
| ^^^^^^^^^^^^^
|
error: `overriden` should be `overridden`
--> tests/test_remote_access.py:104:22
|
104 | """To be overriden by descendants"""
| ^^^^^^^^^
|
After this it should be ok.
59dd770 to
27f6d7e
Compare
folmos-at-orange
approved these changes
Mar 17, 2025
32620bf to
4a936d8
Compare
Collaborator
Author
|
@folmos-at-orange : Please check commit 8e6c19f. |
This was
linked to
issues
Mar 18, 2025
4a936d8 to
774517d
Compare
folmos-at-orange
approved these changes
Mar 19, 2025
- drop deprecated arguments - add new arguments - deprecate results_dir - drop all previous v11 deprecations
If a log file path is supplied by the user, then keep the log for subsequent user inspection.
…aries Deprecate previous data-path '`'-based convention.
Indeed, backward compatibility with pre-v11 Khiops versions has been dropped.
Thusly, stop disambiguating Khiops paths, as the API mode ensures that paths can only be relative to the current working directory.
Only keep list-like, convertible to NumPy arrays, input observables.
- propagate Core API updates to parameter passing tests - propagate secondary-table path update to sklearn tests
- update Core API calls to the v11 Core API - drop all sklearn-specific deprecated Khiops estimator fields - drop deprecated `key` dataset attribute - take into account dataset deprecation drops - drop deprecated sklearn samples
Thus, the baseline model (starting with (B_)) is taken account of, but is not used (so that regressions are not entailed).
See the comment KhiopsML/khiops#626 (comment) of Khiops core issue KhiopsML/khiops#626 for the full rationale.
Also update the testing Docker images accordingly.
774517d to
c624616
Compare
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
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.
Closes #325, #329, #236, #241, #81, #77.
Supersedes PR #366 .
TODO Before Asking for a Review
dev(ormainfor release PRs)Unreleasedsection ofCHANGELOG.md(no date)index.html