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

Lots of untested doctest code #104

Closed
WSDeWitt opened this issue Jul 20, 2023 · 1 comment
Closed

Lots of untested doctest code #104

WSDeWitt opened this issue Jul 20, 2023 · 1 comment

Comments

@WSDeWitt
Copy link
Contributor

WSDeWitt commented Jul 20, 2023

@jgallowa07, PR #103 makes a ton of very minor changes, mostly to docs, but some restructuring of GH workflows to simplify and also increase what is tested. A major problem I'm having is that almost none of the doctest code throughout the package (the many examples in docstrings) runs after switching on testing of the docstring examples (running pytest --doctest-modules). My impression is that, like the notebooks, it turns out this code is not tested at all. To avoid breaking this code, each time a developer changes the API, they would have to stare at all these docstring code examples in every file to reason about what needs changing. I don't think that's how we want to work, so I recommend fixing the doctests so that they actually run, and checking them in CI. While one of the Jupyter notebooks has a long runtime that arguably warrants not checking it in tests, we should design docstring examples that are actually runnable and that we can check.

I made some progress toward the doctests, but it's going to need another chunk of work, and I won't be available for that in the near term. I think what I'll do is retreat from doctest in #103, leaving the GH action line commented out, and we can work the PR in without that, and later (hopefully soon) someone can fix the docstrings.

@WSDeWitt WSDeWitt changed the title Lots of untested docstest code Lots of untested doctest code Jul 20, 2023
WSDeWitt added a commit that referenced this issue Jul 20, 2023
@WSDeWitt
Copy link
Contributor Author

I decided to go ahead and fix this, instead of just complaining. Doctests have been revised in #103

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

No branches or pull requests

2 participants