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

Ci/run doctests #1347

Merged
merged 11 commits into from
Mar 3, 2025
Merged

Ci/run doctests #1347

merged 11 commits into from
Mar 3, 2025

Conversation

Flix6x
Copy link
Contributor

@Flix6x Flix6x commented Mar 2, 2025

Description

  • The CI now runs the doctests in our utils subpackage
  • The CI now runs the doctests in our data/schemas subpackage
  • Added changelog item in documentation/changelog.rst

Look & Feel

Here's an example doctest we have:

def parameterize(word):
    """Parameterize the word, so it can be used as a Python or JavaScript variable name.
    For example:
    >>> parameterize("Acme® EV-Charger™")
    'acme_ev_chargertm'
    """
    return inflection.parameterize(word).replace("-", "_")

How to test

The CI takes care of this.

Further Improvements

Some of our other subpackages have some doctests, too. Some of them require app contexts and I couldn't get that to work easily. Most of them also appear to be meant more like an example than as a unit test. I usually only made use of doctests as unit tests for util functions.

@Flix6x Flix6x self-assigned this Mar 2, 2025
@Flix6x Flix6x added this to the 0.25.0 milestone Mar 2, 2025
@Flix6x Flix6x marked this pull request as ready for review March 2, 2025 20:39
@Flix6x Flix6x requested a review from nhoening March 2, 2025 20:39
Copy link
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

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

Why are we not actually running the doctests you have fixed here in these two modules?

  • data/models/data_sources
  • data/services/utils

@Flix6x
Copy link
Contributor Author

Flix6x commented Mar 3, 2025

Why are we not actually running the doctests you have fixed here in these two modules?

  • data/models/data_sources
  • data/services/utils

One is actually passing, but the other is just serving as an example. In general, some of our docstrings contain examples that can't actually run without an app context.

Should I add a CI step for just the one extra docstring that is passing as a doctest?

@nhoening
Copy link
Contributor

nhoening commented Mar 3, 2025

Should I add a CI step for just the one extra docstring that is passing as a doctest?

No, only if we might add some more there.

@Flix6x
Copy link
Contributor Author

Flix6x commented Mar 3, 2025

Any other changes requested, or would you approve this PR?

@nhoening
Copy link
Contributor

nhoening commented Mar 3, 2025

I approve

@Flix6x Flix6x dismissed nhoening’s stale review March 3, 2025 16:30

He approves.

@Flix6x Flix6x merged commit 3d5cbf5 into feat/units/shared-base-units Mar 3, 2025
10 checks passed
@Flix6x Flix6x deleted the ci/run-doctests branch March 3, 2025 16:30
Flix6x added a commit that referenced this pull request Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants