Skip to content
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

Remove astpretty as a dependency of pyflamegpu #1166

Merged
merged 2 commits into from
Dec 15, 2023
Merged

Conversation

ptheywood
Copy link
Member

astpretty is only used for prettyprint debugging in codegen testing - it is not a pyflamegpu dependency.

This commit also makes it an optional test_codegen.py dependency (but installs it in the test venv by default)

Closses #1164

astpretty is only used for prettyprint debugging in codegen testing - it is not a pyflamegpu dependency.

This commit also makes it an optional test_codegen.py dependency (but installs it in the test venv by default)

Closses #1164
astpretty is only used for prettyprint debugging in codegen testing - it is not a pyflamegpu dependency.

This commit also makes it an optional test_codegen.py dependency (but installs it in the test venv by default)

Closses #1164
@ptheywood
Copy link
Member Author

ptheywood commented Dec 14, 2023

pytest codegen tests still pass:

but if ran without astpretty installed, a message is output to the debug info (i.e. if someone doesn't build pyflamegpu, but installs pytest themselves and runs the test suite, so unlikely but no reason not to support it.)

python3 -m pytest ../tests/python/codegen/ -s
... 
collecting ... Install astpretty if ast tree printing required for test debugging
...
=========== 57 passed in 0.30s ==========

# install pytest
COMMAND ${VENV_PIP} install pytest
# install pytest and astpretty
COMMAND ${VENV_PIP} install pytest astpretty
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to remove it from the venv, if it's used by the codegen tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is adding it to the venv if pytest is also being installed.

Previously it would be implicitly installed as a dependency of pyflamegpu.

The changes to test_codegen.py itself making it optional within the test is not really neccesarry, just adds a bit more flexibility, it makes it so that just pyflamegpu and pytest are required, but if codegen debugging is wanted then astpretty might help.

@ptheywood ptheywood merged commit a4fb913 into master Dec 15, 2023
20 checks passed
@ptheywood ptheywood deleted the remove-astpretty-dep branch December 15, 2023 11:16
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