Skip to content

fix: use sysconfig to locate khiops_env.cmd on Windows#592

Merged
popescu-v merged 7 commits into
mainfrom
try_fix_windows_without_env
Jun 30, 2026
Merged

fix: use sysconfig to locate khiops_env.cmd on Windows#592
popescu-v merged 7 commits into
mainfrom
try_fix_windows_without_env

Conversation

@bruno-at-orange

@bruno-at-orange bruno-at-orange commented Jun 26, 2026

Copy link
Copy Markdown
Member

On Windows without a virtual environment, sys.executable points to the Python binary directory (e.g. C:\Python312) while pip installs console scripts into the Scripts\ subdirectory. Using Path(sys.executable).parent therefore missed khiops_env.cmd.

Replace with sysconfig.get_path("scripts") which always returns the correct Scripts directory for the active Python environment, covering virtual envs, system installs, and user installs. Also check the user-scheme scripts dir (nt_user) as a fallback for pip install --user.

Put your message here


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

@bruno-at-orange bruno-at-orange force-pushed the try_fix_windows_without_env branch from 7d4e314 to 1de746b Compare June 29, 2026 07:42
@popescu-v popescu-v force-pushed the try_fix_windows_without_env branch 11 times, most recently from b36add3 to d0607c6 Compare June 30, 2026 13:29
@popescu-v popescu-v requested review from tramora and removed request for popescu-v June 30, 2026 13:30
@popescu-v

Copy link
Copy Markdown
Collaborator

The first 2 commits authored by @bruno-at-orange LGTM.

@tramora tramora left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No blocking remarks.

Comment thread .github/workflows/pip.yml
# hence, it is hard-coded
image: |-
ghcr.io/khiopsml/khiops-python/khiopspydev-${{ matrix.container }}:${{ inputs.image-tag || 'latest' }}
env:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a reader I would like to know why for a specific env I will have only "use-virtualenv: true" test for example. Is it the right place here to add a comment for each item ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment added.

Comment thread .github/workflows/pip.yml
Comment thread .github/workflows/pip.yml Outdated
Comment thread khiops/core/internals/runner.py Outdated
@popescu-v popescu-v force-pushed the try_fix_windows_without_env branch from d0607c6 to 588f8f6 Compare June 30, 2026 15:43
@popescu-v popescu-v requested a review from tramora June 30, 2026 15:43
@popescu-v popescu-v force-pushed the try_fix_windows_without_env branch from 588f8f6 to fab5295 Compare June 30, 2026 15:56
bruno-at-orange and others added 7 commits June 30, 2026 17:57
On Windows without a virtual environment, sys.executable points to
the Python binary directory (e.g. C:\Python312\) while pip installs
console scripts into the Scripts\ subdirectory. Using
Path(sys.executable).parent therefore missed khiops_env.cmd.

Replace with sysconfig.get_path("scripts") which always returns the
correct Scripts directory for the active Python environment, covering
virtual envs, system installs, and user installs. Also check the
user-scheme scripts dir (nt_user) as a fallback for pip install --user.
…elper

- Replace if/elif chain with a match/case on installation_method for
  clarity; each case now owns its full lookup logic
- Extract _infer_khiops_env_from_path() helper (shutil.which + error)
  used by 'conda' and 'pip'-on-Unix cases
- Fix 'conda' and 'pip'+Unix cases: return value of helper was
  previously discarded, leaving khiops_env_path unbound
- Fix 'pip' case: restore missing distribution('khiops-core') check
- pip + Windows: use sysconfig.get_path('scripts') / 'nt_user' scheme
  determined by whether this library lives under user site-packages,
  avoiding a blind directory search and correctly handling venv,
  system-wide and --user installs
…tests

Thests are now run using the `khiops-core` Pip package which ships with
the MODL* binaries.
…tualenv

Use platforms (GitHub runners, Docker images) that are pristine with
respect to Khiops (no native packages or any Khiops-specific support
installed.
impi-rt is not present on TestPyPI, whence the installation fails
otherwise.
Put comments before switching logic for consistency with the code base.
@popescu-v popescu-v force-pushed the try_fix_windows_without_env branch from fab5295 to 62c02fe Compare June 30, 2026 15:58
@popescu-v popescu-v merged commit 5372717 into main Jun 30, 2026
25 checks passed
@popescu-v popescu-v deleted the try_fix_windows_without_env branch June 30, 2026 16:27
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.

3 participants