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: Add docstrings for fixtures defined from pandas.tests.io.conftest #57047

Closed
wants to merge 2 commits into from

Conversation

jordan-d-murphy
Copy link
Contributor

@jordan-d-murphy jordan-d-murphy commented Jan 24, 2024

Add docstrings for fixtures defined from pandas.tests.io.conftest

@jordan-d-murphy jordan-d-murphy changed the title Add docstrings for fixtures defined from pandas.tests.io.conftest DOC: Add docstrings for fixtures defined from pandas.tests.io.conftest Jan 24, 2024
@jordan-d-murphy
Copy link
Contributor Author

@datapythonista would you please give this PR a review and share any feedback?

@datapythonista
Copy link
Member

Is there any validation for it we are trying to enforce? I personally don't see much value here otherwise. It'd be nice to have proper documentation for fixtures for sure, but that would take understanding well them and providing useful information on what they are exactly and when they can be used. This PR seems to simply repeat the fixture names in a more verbose way. I'd prefer to not have them rather than this.

@datapythonista datapythonista added Testing pandas testing functions or related to the test suite Docs labels Jan 25, 2024
@mroeschke
Copy link
Member

Agreed with @datapythonista on the value of this. If you're interested, continuing to enforce numpydoc validations would be worthwhile: https://numpydoc.readthedocs.io/en/latest/validation.html#built-in-validation-checks

@jordan-d-murphy
Copy link
Contributor Author

jordan-d-murphy commented Jan 25, 2024

@datapythonista @mroeschke

Thank you gentlemen, I appreciate the feedback and guidance!

One way I do think this adds value is when running pytest --fixtures pandas the output before these changes is:

before

and after adding the docstrings:

after

Considering this, is this meaningful to add? If not, I'm happy to revert these changes and move on to working on continuing to enforce numpydoc validations as Matthew suggested.

@mroeschke
Copy link
Member

Considering this, is this meaningful to add?

Just to make my last comment clearer - I would still be OK with documenting these pytest fixtures, but I don't think there's a ton of value (impact) in doing so.

@jordan-d-murphy
Copy link
Contributor Author

@mroeschke I've opened #57111 to continue work on enforcing numpydoc validations: https://numpydoc.readthedocs.io/en/latest/validation.html#built-in-validation-checks

@jordan-d-murphy jordan-d-murphy deleted the issue#19159 branch January 28, 2024 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants