DOC/CI: Fixes to make validate_docstrings.py to not generate warnings or unwanted output #23552
Conversation
|
Hello @datapythonista! Thanks for submitting the PR.
|
|
Changes look good to me.
Can you remind me what the end-goal of this is? |
|
The json file is for me (or anyone else interested), to know what needs to be fixed in the docstrings. The end goal is to have My next PR will be something like adding |
Codecov Report
@@ Coverage Diff @@
## master #23552 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 161 161
Lines 51224 51224
=======================================
Hits 47254 47254
Misses 3970 3970
Continue to review full report at Codecov.
|
| @@ -1875,35 +1875,31 @@ def get_duplicates(self): | |||
|
|
|||
| Works on different Index of types. | |||
|
|
|||
| >>> pd.Index([1, 2, 2, 3, 3, 3, 4]).get_duplicates() | |||
| >>> pd.Index([1, 2, 2, 3, 3, 3, 4]).get_duplicates() # doctest: +SKIP | |||
There was a problem hiding this comment.
Why does this need to be skipped?
There was a problem hiding this comment.
Ah, you are skipping the deprecated things, to avoid warnings?
There was a problem hiding this comment.
yes, that's correct
| 1 ba | ||
| 2 -e | ||
| 3 dc | ||
| dtype: object |
There was a problem hiding this comment.
I think here it is actually useful to see the difference? (although it is deprecated?)
cc @h-vetinari
There was a problem hiding this comment.
I think it makes more sense to remove it. But if we restore it, I think the page should make clear that this is a deprecated behavior. The way it was feels like it's encouraging users to use join=None.
There was a problem hiding this comment.
Yes, that is true that it then should be more clear that it is deprecated, and pointing out the difference.
There was a problem hiding this comment.
@datapythonista @jorisvandenbossche
Didn't see this on time I guess.
Indeed, the main intention of this was to show the difference, while relying on the fact that the user would see the deprecation warning if he actually used the code (but then, that was before all that docstring-validation jazz ;-)).
So, I think that example should go back in, albeit with a clearer note about the deprecation. How to avoid problems with the emitted warning? A # doctest: +SKIP marker like below?
| @@ -206,7 +206,7 @@ def radviz(frame, class_column, ax=None, color=None, colormap=None, **kwds): | |||
| ... 'versicolor', 'setosa', 'virginica', | |||
| ... 'setosa'] | |||
| ... }) | |||
| >>> rad_viz = pd.plotting.radviz(df, 'Category') | |||
| >>> rad_viz = pd.plotting.radviz(df, 'Category') # doctest: +SKIP | |||
There was a problem hiding this comment.
But is the plot still shown then in the html documentation?
There was a problem hiding this comment.
Yes, it's just skipped in the doctests. Just generated the page to confirm, and it's rendered as expected.
There was a problem hiding this comment.
But there are many other plots in the docs? Why only those two? And from reading further it seems you are deactivating matplotlib anyway in the script?
There was a problem hiding this comment.
I'm not skipping because it's a plot, I'm skipping because it generates a warning, something about the projection (not sure if the warning only happens with the backend Template)
There was a problem hiding this comment.
OK. I don't see any warning locally, though
(the problem with skipping it in general is that you don't catch typo's or other mistake in this line when using the validation script)
|
I agree that skipping is not ideal, but I think in this case is better than having the warnings, and I don't see other options. But if you prefer, I can remove the |
|
It's fine for this one. I just think we might need to think of a better general way to deal with this. As there can also be genuine cases where you want a warning (showing a use case that gives a certain warning). It would be annoying to always have to skip those. |
|
Totally agree. I think all the skips except the plot one are for deprecations. And personally I think it's reasonable to skip them since the deprecation until the removal. The plot one would be probably worth investigating. But with all the work pending in the docstrings, I prefer to skip that test, and move forward with all the fixes. Let me know if this can be merged, or if there is anything that needs to be changed. |
…fixed * upstream/master: (47 commits) CLN: remove values attribute from datetimelike EAs (pandas-dev#23603) DOC/CI: Add linting to rst files, and fix issues (pandas-dev#23381) PERF: Speeds up creation of Period, PeriodArray, with Offset freq (pandas-dev#23589) PERF: define is_all_dates to shortcut inadvertent copy when slicing an IntervalIndex (pandas-dev#23591) TST: Tests and Helpers for Datetime/Period Arrays (pandas-dev#23502) Update description of Index._values/values/ndarray_values (pandas-dev#23507) Fixes to make validate_docstrings.py not generate warnings or unwanted output (pandas-dev#23552) DOC: Added note about groupby excluding Decimal columns by default (pandas-dev#18953) ENH: Support writing timestamps with timezones with to_sql (pandas-dev#22654) CI: Auto-cancel redundant builds (pandas-dev#23523) Preserve EA dtype in DataFrame.stack (pandas-dev#23285) TST: Fix dtype mismatch on 32bit in IntervalTree get_indexer test (pandas-dev#23468) BUG: raise if invalid freq is passed (pandas-dev#23546) remove uses of (ts)?lib.(NaT|iNaT|Timestamp) (pandas-dev#23562) BUG: Fix error message for invalid HTML flavor (pandas-dev#23550) ENH: Support EAs in Series.unstack (pandas-dev#23284) DOC: Updating DataFrame.join docstring (pandas-dev#23471) TST: coverage for skipped tests in io/formats/test_to_html.py (pandas-dev#22888) BUG: Return KeyError for invalid string key (pandas-dev#23540) BUG: DatetimeIndex slicing with boolean Index raises TypeError (pandas-dev#22852) ...
Follow up of #23514. Making
validate_docstrings.pynot generate warnings or other unwanted output, mainly when called with--format=json. This includes preventing matplotlib of opening windows with the plots, and also canceling output from flake8.Also, fixed some bugs, and corrected some tests that weren't being run.
The script should now be ready to be added to the CI (and to generate the json file with the docstrings state of the art).
git diff upstream/master -u -- "*.py" | flake8 --diffCC @TomAugspurger