Skip to content

Compile catalogs to top-level turtle_docstringdict_<lang> modules - #2

Merged
StanFromIreland merged 5 commits into
mainfrom
top-level
Sep 28, 2026
Merged

StanFromIreland merged 5 commits into
mainfrom
top-level

Conversation

@StanFromIreland

@StanFromIreland StanFromIreland commented Sep 26, 2026 •

Copy link
Copy Markdown
Member

And, add support for older versions, via a runtime filter.

I don't think per-version dictionaries are worth the trouble right now, either to generate or to store. Docstrings rarely change, and when they do it is usually an improvement that gets backported anyway. There may be cases where a newer docstring documents a feature an older version doesn't have, but I don't think that's common enough yet to justify the machinery. We can revisit it in a follow-up issue or discussion.

This PR instead compiles each catalogue to a single top-level turtle_docstringdict_<lang>.py module, and on
import, the module drops any entry whose name doesn't exist in the running version of turtle.

WDYT @m-aciek ?

@m-aciek m-aciek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This technically looks good to me (except for the missing test), though I think it is really worth it to cover more versions than just the latest, for the start, especially in light of docstring cleaning, that you recently started. Educators rarely use latest versions of the language, they often are behind.

I'm ok to rebase the other PR onto this one for more gradual commit history on main, but I believe we shouldn't abandon the wider support idea.

Comment thread scripts/i18n.py Outdated
Comment thread scripts/i18n.py
Comment on lines +120 to +127
import turtle

for _key in list(docsdict):
_obj = turtle
for _attr in _key.split("."):
_obj = getattr(_obj, _attr, None)
if _obj is None:
del docsdict[_key]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If we were to go ahead with this, could we please cover this fragment with a test that ensures it doesn't fail in runtime?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For now, I've added an integration test that runs with 3.13, 3.14 and 3.15 and asserts this works. I'll look into cherry-picking your (more precise) tests from the other PR and including one for this.

Comment thread scripts/i18n.py Outdated
@StanFromIreland

Copy link
Copy Markdown
Member Author

especially in light of docstring cleaning

Note that I will be backporting those changes as far as I can. Additionally in most cases, the translated string would be better than translating the older one.

wider support idea.

I think this gives us the widest support possible, by determining what to remove at runtime, we support all Python 3 versions.

@StanFromIreland
StanFromIreland merged commit 8161706 into main Sep 28, 2026
6 checks passed
@StanFromIreland
StanFromIreland deleted the top-level branch September 28, 2026 08:46
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