Skip to content

fix(quantization): use raw docstrings for '\*' escapes - #143

Merged
dengqiaoyu merged 4 commits into
apple:mainfrom
alphasafal:fix/invalid-escape-sequences
Nov 4, 2026
Merged

dengqiaoyu merged 4 commits into
apple:mainfrom
alphasafal:fix/invalid-escape-sequences

Conversation

@alphasafal

Copy link
Copy Markdown
Contributor

The OpQuantizerConfig and ModuleQuantizerConfig docstrings escape * as \* so Sphinx doesn't treat it as emphasis. In a normal string literal that's an invalid escape sequence, so on Python 3.12+ compiling quantization_config.py prints

SyntaxWarning: invalid escape sequence '\*'

and with python -W error (or a pytest config that turns warnings into errors) the module doesn't compile at all. I ran into it right after installing on 3.13.

Changes

  • Make both docstrings raw. The string values are identical to before (the backslash is kept either way), so the rendered docs don't change. I compared the AST before and after to be sure.
  • Same r prefix on four regex literals in test_annotation_pattern_registry.py that had \. / \*.
  • Enable ruff W605 so make check catches new ones. On main it flags exactly these 20 spots.
  • Add tests/test_source_compiles.py, which compiles every module under coreai_opt with warnings as errors. It fails on main and passes with this change.
  • Changelog fragment under Fixed.

Testing

  • pre-commit run --all-files passes.
  • pytest -m "not slow" on Python 3.13 / Linux: everything passes apart from export and nox tests that need coremltools, pyreporoot and model downloads I couldn't install in my environment. None of those touch the files changed here.

@alphasafal

Copy link
Copy Markdown
Contributor Author

@dengqiaoyu friendly ping when you have a moment. This removes the Python 3.12+ SyntaxWarnings and adds a test so they can't come back.

@dengqiaoyu
dengqiaoyu self-requested a review October 29, 2026 18:18
@dengqiaoyu

Copy link
Copy Markdown
Contributor

friendly ping when you have a moment. This removes the Python 3.12+ SyntaxWarnings and adds a test so they can't come back.

Thanks for the reminder, will do. I was not added as a reviewer so I didn't get any notification for it.

@dengqiaoyu dengqiaoyu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, please remove the change log then I will approve it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is no need to add a changelog for the small fix.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the tests.

@alphasafal
alphasafal force-pushed the fix/invalid-escape-sequences branch 3 times, most recently from 2b238e5 to eb18ca3 Compare November 3, 2026 17:42
@dengqiaoyu

Copy link
Copy Markdown
Contributor

@alphasafal Can you also configure your GitHub account with signing key so that your commits are signed and we can merge it? Thanks!

6212 60594

@alphasafal
alphasafal force-pushed the fix/invalid-escape-sequences branch from eb18ca3 to d82df9e Compare November 3, 2026 17:47
@alphasafal

Copy link
Copy Markdown
Contributor Author

Thanks @dengqiaoyu! I’ve set up my signing key, and all 4 commits now show as Verified. I’ve also removed the changelog fragment and rebased onto the latest main. Ready for another look.

@dengqiaoyu
dengqiaoyu enabled auto-merge (squash) November 3, 2026 18:06
@alphasafal
alphasafal requested a review from dengqiaoyu November 3, 2026 19:18
@dengqiaoyu
dengqiaoyu merged commit e8427f3 into apple:main Nov 4, 2026
14 checks passed
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