Skip to content

Improve docstring test legibility for junior devs #1750

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

Merged

Conversation

pushfoo
Copy link
Member

@pushfoo pushfoo commented May 5, 2023

Changes

tl;dr Improves the state of #1570 by outputting the example docstring spec on failure & generally improving test clarity

  • Add a brief explanation of the expected example docstring format to test failures by expanding the relevant function docstring
  • Use Path.read_text to immediately close source files after read
  • Rearrange functions in dependency order
  • Rename variables for clarity
  • Add comments

How to test

  1. Delete python -m line from any example(s) in arcade/examples or one of its immediate child folders, except for perf_test and internationalization example files / folders.
  2. ./make.py test
  3. Check for the full docstring of check_code_docstring in the output

Follow-up work

While working on this PR, I discovered the following problems:

  1. The GUI examples do not match our docstring standard
  2. The GUI examples (arcade.gui.examples.*) are not covered by the test due to the test assuming all examples are in the same folder.
  3. Recursion into deeper submodules (arcade.examples.gl.ray_marching.*) is broken

Generalized Docstring Check Functionality?

I do not know what @eruvanos is planning to do with the example structure. The second two problems may be best handled by refactoring the test to:

  1. fully recurse modules / folders
  2. generalize docstring check functionality into something which takes a callable

@pushfoo pushfoo marked this pull request as draft May 5, 2023 03:42
@pushfoo pushfoo marked this pull request as ready for review May 6, 2023 07:01
@pvcraven pvcraven merged commit b26c736 into pythonarcade:development May 6, 2023
@pushfoo pushfoo deleted the improve_docstring_test_clarity branch May 6, 2023 23:01
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