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

DOC: Fix F821 error in docstring #57863

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Conversation

tqa236
Copy link
Contributor

@tqa236 tqa236 commented Mar 16, 2024

The command below returns no error now.

flake8 --doctest --ignore="" --select=F821 pandas/ | grep -v "F821 undefined name 'pd'" | grep -v "F821 undefined name 'np'"

@datapythonista datapythonista changed the title Fix F821 error in docstring DOC: Fix F821 error in docstring Mar 16, 2024
Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Thanks @tqa236, looks great

@datapythonista
Copy link
Member

@tqa236 should we add a check in the CI for this?

@tqa236
Copy link
Contributor Author

tqa236 commented Mar 19, 2024

Hello @datapythonista, I fully agree that fixing linting errors without enforcing them is quite inefficient. But I wonder if the CI check is a long term solution as it's based on flake8 --doctest while we're moving more and more to ruff.

There's an open issue on ruff to track this feature: astral-sh/ruff#3542

In my opinion, we can wait for the feature to be implemented in ruff and enable it here rather than implementing our own check with flake8 --doctest for 2 reasons

  • The code in all of the official docstrings are already run and checked if there's any syntax errors already.
  • What's left for this CI check is only the internal docstring (in conftest,... ), and the number of errors is quite low (it in fact reduces compared to your result last year from the issue).

Let me know what you think.

@datapythonista
Copy link
Member

Sounds good, up to you, I was just wondering if you considered it. Thanks for all the details and for the continued work on this.

@mroeschke mroeschke added this to the 3.0 milestone Mar 19, 2024
@mroeschke mroeschke merged commit b9be19b into pandas-dev:main Mar 19, 2024
46 checks passed
@mroeschke
Copy link
Member

Thanks @tqa236

@tqa236 tqa236 deleted the fix-docstring-error branch March 19, 2024 19:54
pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
Fix F821 error in docstring

Co-authored-by: Marc Garcia <garcia.marc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants