Skip to content

fix: use pathlib.Path for cross-platform path parsing on Windows#6044

Open
andrey-savov wants to merge 9 commits intoopen-edge-platform:developfrom
andrey-savov:fix/windows-path-handling-issue-4490
Open

fix: use pathlib.Path for cross-platform path parsing on Windows#6044
andrey-savov wants to merge 9 commits intoopen-edge-platform:developfrom
andrey-savov:fix/windows-path-handling-issue-4490

Conversation

@andrey-savov
Copy link
Copy Markdown

Summary

  • Replace recipe.split("/")[-2] with Path(recipe).parts[-2] in list_models() — fixes IndexError when running otx find on Windows where str(Path) uses backslashes
  • Replace string split/join pattern in OTXEngine.from_model_name() with Path(default_config).parent / f"{model_name}.yaml" for the same reason

Fixes #4490.

Test plan

  • uv run pytest library/tests/unit/backend/native/utils/test_api.py — 11 tests pass
  • uv run pytest library/tests/unit/backend/native/test_engine.py — 15 tests pass
  • Manual: otx find --task MULTI_CLASS_CLS on Windows no longer raises IndexError

Copilot AI review requested due to automatic review settings April 4, 2026 17:58
@andrey-savov andrey-savov requested a review from a team as a code owner April 4, 2026 17:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Windows path-handling issues in the native backend by switching from manual string splitting/joining to pathlib.Path-based path parsing and construction, preventing IndexError in otx find and improving cross-platform correctness.

Changes:

  • Update list_models() to derive the task directory via Path(recipe).parts instead of recipe.split("/").
  • Update OTXEngine.from_model_name() to build the target config path via Path(default_config).parent / f"{model_name}.yaml" instead of split/join on /.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
library/src/otx/backend/native/engine.py Builds model config paths using Path(...).parent / ... to avoid platform-specific separators.
library/src/otx/backend/native/cli/utils.py Uses Path(recipe).parts to extract the task segment in a platform-independent way for CLI table output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread library/src/otx/backend/native/engine.py
Comment thread library/src/otx/backend/native/cli/utils.py
@andrey-savov
Copy link
Copy Markdown
Author

Tested end-to-end on Windows 11 with \ — now works fully. Also fixed a second Windows-only bug discovered during testing: \ when Rich tried to render the OTX logo through the legacy Windows console (cp1252 encoding can't handle the box-drawing characters). Fix: reconfigure stdout to UTF-8 at CLI entry and disable Rich's legacy Windows renderer ().

andrey-savov added a commit to andrey-savov/training_extensions that referenced this pull request Apr 4, 2026
@github-actions github-actions bot added the DOC Improvements or additions to documentation label Apr 4, 2026
- Replace recipe.split('/') with Path(recipe).parts in list_models() to
  fix IndexError in 'otx find' on Windows (fixes open-edge-platform#4490)
- Replace string split/join in OTXEngine.from_model_name() with
  Path.parent / name for cross-platform correctness
- Add None guard in from_model_name() with helpful error listing
  supported tasks (addresses review comment)
- Cache Path(recipe) per loop iteration in list_models() (addresses
  review comment)
- Reconfigure stdout to UTF-8 and disable Rich legacy Windows renderer
  to fix UnicodeEncodeError when printing the OTX logo on Windows
- Update CHANGELOG
@andrey-savov andrey-savov force-pushed the fix/windows-path-handling-issue-4490 branch from 73e8281 to 4a4bedc Compare April 4, 2026 20:49
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
library/src/otx/backend/native/engine.py 40.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@andrey-savov
Copy link
Copy Markdown
Author

Added a test case in est_from_config_with_model_name that passes an unsupported task to OTXEngine.from_model_name(), covering the previously missing ValueError branch. All 3 uncovered lines in �ngine.py now have coverage. Commit: 4f5581b.

@github-actions github-actions bot added the TEST Any changes in tests label Apr 7, 2026
@andrey-savov andrey-savov requested a review from a team as a code owner April 7, 2026 14:38
@github-actions github-actions bot added the BUILD label Apr 7, 2026
@kprokofi kprokofi requested review from itallix and warrkan April 8, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BUILD DOC Improvements or additions to documentation TEST Any changes in tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't find tasks: IndexError

3 participants