Skip to content

Conversation

jbrockmendel
Copy link
Member

No description provided.

@jbrockmendel
Copy link
Member Author

@datapythonista is there an easy way to run the docstring validation on non-public/non-API functions? here im thinking of tslibs.ccalendar

(side-note, does the docstring validation check that type annotations match docstring parameters and return types?)

@datapythonista
Copy link
Member

The script runs on all public functions/methods when called without parameters, but it can run on private functions if you pass it as parameter (e.g. ./scripts/validate_docstrings.py pandas.Series._private_method).

We don't check type annotations at the moment, but I guess shouldn't be difficult to implement it. The script now lives in numpydoc, I'm already working on replacing our version by the one in numpydoc.

@jbrockmendel
Copy link
Member Author

@WillAyd @mroeschke this is pretty spread out. LMK if there are pieces I can split out that you'd be comfortable merging.

return validator.validate(values)


# TODO: only non-here use is in test
Copy link
Member

Choose a reason for hiding this comment

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

If so, I'm okay with removing in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Poor wording on my part. This function is used inside lib.pyx, but its only non-cython use in in tests. So the option is to make it cdef instead of cpdef at the cost of not testing it directly.

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay. Thanks for clarifying that.

return is_timedelta(value)


# TODO: only non-here use is in test
Copy link
Member

Choose a reason for hiding this comment

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

Same

"_libs.properties": {"pyxfile": "_libs/properties", "include": []},
"_libs.reshape": {"pyxfile": "_libs/reshape", "depends": []},
"_libs.skiplist": {
"pyxfile": "_libs/skiplist",
Copy link
Contributor

Choose a reason for hiding this comment

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

delete skiplist.h?

Copy link
Member Author

Choose a reason for hiding this comment

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

still needed



def _validate_limit(nobs: int, limit=None) -> int:
if limit is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a doc-string

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, next pass

@jreback jreback added the Clean label Nov 2, 2019
@jreback jreback added this to the 1.0 milestone Nov 2, 2019
@jreback
Copy link
Contributor

jreback commented Nov 2, 2019

looks fine, happy to merge now and follow up with those 2 issues or repush, lmk.

@jreback jreback merged commit 221c8a7 into pandas-dev:master Nov 2, 2019
@jreback
Copy link
Contributor

jreback commented Nov 2, 2019

thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the ref-libs branch November 2, 2019 15:30
jbrockmendel added a commit to jbrockmendel/pandas that referenced this pull request Nov 2, 2019
jreback pushed a commit that referenced this pull request Nov 2, 2019
Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
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.

4 participants