Skip to content

Fix Windows path handling in repo checkers (check_auto, check_noisy_comments) - #48952

Open
dltsum wants to merge 1 commit into
huggingface:mainfrom
dltsum:fix-repo-checkers-windows
Open

dltsum wants to merge 1 commit into
huggingface:mainfrom
dltsum:fix-repo-checkers-windows

Conversation

@dltsum

@dltsum dltsum commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

CPU CI GPU run-slow

What

Running the repo-utils test suites on Windows surfaced three forward-slash assumptions:

  1. check_auto.build_config_mapping_namesconfig_path.split("/")[-2] extracts the module directory from glob results, but glob.glob returns OS-native separators. On Windows module_name is never the directory name, so model_type == module_name is never true: every config counts as a non-natural match, and a non-natural alias (e.g. MaskFormerDetrConfig with model_type="detr") overwrites the canonical DetrConfig. This makes the generated auto-mapping content order-dependent on Windows. → os.path.basename(os.path.dirname(...)).

  2. check_noisy_comments._display_path — findings are printed with OS-native separators, so CI logs (and the tests asserting on them) differ by platform. → print POSIX-style paths via .as_posix().

  3. test_conversion_orderFILES_TO_PARSE is built with os.path.join, then split on "/", raising IndexError on Windows. → same basename(dirname()) extraction.

Verification

Windows 11, cp936 locale, Python 3.14:

  • Before: tests/repo_utils/test_check_auto.py 4 failed, test_check_noisy_comments.py 1 failed, modular/test_conversion_order.py 1 failed.
  • After: all three files green — 40 passed.

🤖 Generated with Claude Code

Three forward-slash assumptions that break on Windows, all surfaced by
running the repo-utils test suites on Windows (cp936):

- check_auto.build_config_mapping_names: glob returns OS-native
  separators, so config_path.split("/")[-2] never yields the module
  directory name; every config then counts as a non-natural match and a
  non-natural alias (e.g. MaskFormerDetrConfig for model_type=detr) can
  overwrite the canonical class. Use os.path.basename(os.path.dirname()).
- check_noisy_comments._display_path: printed paths used OS separators,
  making CI log output differ on Windows; print POSIX-style paths.
- test_conversion_order: FILES_TO_PARSE is built with os.path.join, so
  splitting on "/" raises IndexError on Windows.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 35441864602
Result: success | Grafana metrics are not available yet.

@Rocketknight1

Copy link
Copy Markdown
Member

cc @tarekziade more stuff that might be automatable with a Ruff rule rather than drip-feeding lots of small fix PRs

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.

2 participants