Report reaching into third-party internals - #708
Open
ecomodeller wants to merge 2 commits into
Open
ecomodeller wants to merge 2 commits into
ecomodeller wants to merge 2 commits into
Conversation
Two ways modelskill can depend on another package's internals: importing a private name, and reading a private attribute off one of its objects. Neither is covered by any deprecation policy, so both break without warning on a patch release. Imports are caught by ruff's PLC2701, which ignores relative imports inside our own package. It needs preview mode; src/ is already clean under it. Attribute access has no equivalent rule -- SLF001 flags all 75 occurrences in src/ without knowing whose object it is, and 74 of those are modelskill reading its own attributes, which is fine. tools/check_third_party_private_access.py narrows SLF001 by name: a member we define somewhere in src/modelskill is ours, anything else belongs to another package. That leaves one finding, ds_column._zn on a mikeio Dataset in dfsu.py. The lint job now runs just, so CI and local dev use the same commands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
relative_to raised ValueError for any path outside the repo and the handler swallowed it silently. is_relative_to asks the question directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reaching into another package's internals — importing a private name, or reading a private attribute off one of its objects — is outside any deprecation policy and breaks on a patch release, with nothing in CI to notice.
Imports are covered by ruff's
PLC2701, which correctly ignores relative imports inside our own package. It is still a preview rule, sopreview = trueis needed;src/is already clean under it.Attribute access has no equivalent.
SLF001is not type-aware — it reports all 75 occurrences insrc/without knowing whose object it is, and 74 are modelskill reading its own attributes, which is ordinary intra-package access.tools/check_third_party_private_access.pynarrowsSLF001by name: a member defined somewhere insrc/modelskillis ours, anything else belongs to another package. Name-based, not type inference — the script's docstring states what it misses.That leaves one finding,
ds_column._znon a mikeioDataset:This must merge after #707, which removes that line. Until then
just private-accessexits 1 and the lint job is red.The lint job now runs
just lintinstead of duplicating the ruff invocation inline, so CI and local dev stay in step.🤖 Generated with Claude Code