Skip to content

python: Tighten stubtest and close generated-API type leaks - #156

Open
bact wants to merge 4 commits into
JPEWdev:mainfrom
bact:type-leaks
Open

bact wants to merge 4 commits into
JPEWdev:mainfrom
bact:type-leaks

Conversation

@bact

@bact bact commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator
  • Add CLOSED and ONTOLOGY to reserved words
  • Stub abstract methods, JSON-LD/RDF classes, and CONTEXT_URLS
  • Fix VERSION type
  • Add __all__ to avoid reexport of rdflib and Python's own standard library

--

Also replace @functools.total_ordering with explicit __le__, __gt__, __ge__:

  • CPython 3.9 and 3.10 synthesize these with a NotImplemented=NotImplemented default argument.
  • CPython 3.11+ do not.
  • So no single stub signature satisfies both.
  • Writing them explicitly lets the stub and runtime agree on all of 3.9-3.14, and removes their allowlist entries.

Allowlist dropped from 82 to 0.

--

model.pyi.j2 stub template has to mirror model.py.j2 template. Tests are added to capture the drift during development.

- Stub abstract methods, JSON-LD/RDF classes, and `CONTEXT_URLS`
- Fix `VERSION` type
- Add `__all__` to reduce export from 122 to 79
- Drop allowlist from 82 to 0

Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
@bact bact added the bug Something isn't working label Sep 21, 2026
@bact
bact marked this pull request as draft September 21, 2026 07:23
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
@github-actions

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  src/shacl2code/lang
  python.py
Project Total  

This report was generated by python-coverage-comment-action

Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
Signed-off-by: Arthit Suriyawongkul <arthit@gmail.com>
@bact
bact marked this pull request as ready for review September 22, 2026 09:18
@bact
bact requested a review from JPEWdev September 22, 2026 09:25

# fmt: off
"""Format Guard{{ '"' }}{{ '"' }}{{ '"' }}
__all__ = [

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is this enforced? Like if we forget one, or add an extra does it make an error?

@bact bact Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not by Python or by the generated binding itselft, but by the test at test_model_all_matches_public_symbols (new in this PR).

  • stubtest catch missing stub
  • test_model_all_matches_public_symbols catch missing exports or leaking imports

I hope I getting it right this time. We have fixing this several times. The test should help use catching that.
The idea is import * from of the generated binding should only import things specifically from the binding, and not other things like imports (standard lib or rdflib that the binding happens to import for its internal use) or internal variables (prefixed with _).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants