-
-
Notifications
You must be signed in to change notification settings - Fork 310
docs: clarify how to assert absence of function calls using Python AST helper #1130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Hi, the CI failure seems unrelated to the docs change. It fails due to Node engine mismatch (Node 22 vs required Node 24): I see Node 24 was recently fixed in #1127, so a re-run of CI should pass. Thank you! |
|
Yes, the failure is due to Node 22 being used while the dependency requires >=24.12.0. Since Node 24 support was aligned in #1127, a CI re-run should pick up the correct version and pass. |
|
restarting the tests does not work, I updated the branch |
|
I have updated my branch with upstream main (Node 24) and formatted the file using Prettier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please do not make changes to dependencies in a PR to add info to the docs, dependency updates if needed go in a different PR
if you are trying to do multiple contributions, this is why it's highly suggested to not make PRs from the main branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the guidance. I’ve reverted all dependency and CI changes and kept this PR docs-only as suggested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
files package.json and pnpm-lock.yaml are still in the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience. I’ve now rewritten the branch so this PR contains only the documentation change and no dependency files.
src/content/docs/curriculum-help.mdx
Outdated
| To assert that a function call does not exist in the learner’s code, simply negate the helper result: | ||
|
|
||
| ```python | ||
| assert not Node(code).has_call("print") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has_call requires the exact call including arguments. So this should at least include parenthesis -> has_call('print()')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for pointing that out. I’ve updated the examples to use has_call("print()") and block_has_call("print()") as required by the helper.
src/content/docs/curriculum-help.mdx
Outdated
|
|
||
| ```python | ||
| assert not Node(code).has_call("print()") | ||
| assert not Node(code).block_has_call("print()") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
block_has_call does not require the literal call
| assert not Node(code).block_has_call("print()") | |
| assert not Node(code).block_has_call("print") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. I’ve updated the example so has_call uses "print()" and block_has_call uses "print" as suggested.
|
Thanks for the detailed reviews and clarifications. I’ve updated the examples so that: has_call uses the literal call form ("print()") block_has_call uses the non-literal form ("print") as intended The PR is now docs-only, with no dependency or CI changes, and all checks are passing. Please let me know if anything else needs adjustment. |
Checklist:
Closes #1121
Adds documentation explaining how to assert the absence of function calls using the Python AST helper. This helps curriculum authors enforce removal of statements such as
print().