CI: speedup docstring check consecutive runs
State: closed

- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).

This PR brings the runtime of the docstring check CI down to 2-3 minutes from about 20 minutes.

Currently, `check_code.sh docstring` does multiple calls to `validate_docstrings.py` due to various error types that have function exceptions. Each run of `validate_docstrings`takes about 2-3 minutes, leading to the  current 20 minutes runtime just for the docstring checks.

The runtime for consecutive calls is brought down to that of a single call by changing the argument parsing of `validate_docstrings`, adding `--` to separate parameters that previously were separate function calls. Additionally, a cache is added to reuse the parsing results between runs. The cache size should pose no problem, the Python instance ran by `check_code.sh docstring' only reserves about 95MB of memory on my machine.

Labels:
name: Docs
name: CI
description: Continuous Integration

Comments:
> This PR brings the runtime of the docstring check CI down to 2-3 minutes from about 20 minutes.

Wow - my hat is raised above my head

@datapythonista fancy taking a look?
Thanks @dontgoto for making the CI much faster.

While I'm open to getting this merged, I'm a bit unsure about using this approach. In an ideal world we'd like to call validate_docstrings just once for all errors and for all files. This is clearly not the case today, but hopefully we'll eventually get there, and making things significantly more complex for something hopefully lasting only few months may not be worth.

Also, if we want to implement this I think I'd prefer another API where we can simply specify which files and errors to ignore together. Not sure exactly the best way to do this, but something like this should be clearer and simpler IMHO: `./validate_docstring.py --ignore pandas/core.py,PR03 --ignore pandas/frame.py,EX01,EX02`

As I said, I'm not opposed to merge this PR, but I think it's making the validator significantly more complex to understand, which is probably worth for the speedup now, but thinking longer term not so much.

What do you think @dontgoto ?
I think I can change the command line args and their preprocessing quite easily to match your `--ignore` idea, but in the end still run everything sequentially. Your parameter variant is indeed easier to understand.

I agree that making the rest of the code match this kind of structure and ditching the repeated sequential calls would be preferable, but maybe a bit much of a time investment.

I might look into that in the future though, I have an idea for another PR that might make the docstring checks "commit-hookably" fast, but I still need to test it out.
I think implementing what I said is not trivial, but I think just one validate call per function would be enough if we do that, so I think it should be as fast as your implementation here, or maybe even a bit faster.
I pushed a first version of the `--ignore` parameter. In my opinion, the behavior of the CLI parameters is now easier to understand, but the parsing logic in the script got more complex.

Let me know what you think about the changes to the `.py`. I am ambivalent, both versions have their own pain points. 

If we go ahead with the current version, I'll tidy it up and add tests for the parsing. Merge conflicts for docstring exceptions removed from main in the meantime are to be expected.
> think just one validate call per function would be enough if we do that, so I think it should be as fast as your implementation here, or maybe even a bit faster.

Regarding performance, everything outside the cached validate function is basically free. Calls of `main` finish in milliseconds or less once the validate document cache is filled in the first run. So no need to minimize calls of `main` just from the performance angle.

Thanks for the work on this @dontgoto. 

What I had in mind is different to what you implemented. And I think it should be simpler, and reasonably fast.

Regardless of the command line API we implement, we would end up with a list of function names and which errors we need to ignore for them. We have this stored in a variable, and then we call the validator normally: https://github.com/pandas-dev/pandas/blob/main/scripts/validate_docstrings.py#L317

At this point, we have in the result the errors that have been found in the function. If the error is in the list of errors to ignore, we can just remove that from the result. This way we don't need cache, we don't need to much extra complexity, and we call the parsing and the validation just once.

What do you think?
I agree. The version I previously pushed was a half measure, just adding a variant of the CMD parameter required for this solution, but shoehorning it into the old logic. I refactored everything to use the new parameter, simplifying the parsing. I am satisfied with this solution, let me know whether you agree.

For the new CMD parameter `for_error_ignore_functions` I find a mapping of `error: list[funcs_to_ignore]` to be the best fit when considering the maintenance of the exception lists in the `code_checks.sh`. Just the initial formatting changes there are not nice. 

Open for a different name for the parameter though.
Looks great, thanks a lot for the work here @dontgoto 

@jordan-d-murphy @tqa236 do you mind having a look here and sharing any feedback on this change? Thanks!
This is such a refreshing PR! Love to see this. I am in full support of this new approach. I added one suggestion - but leave it up to you if you think it's valuable to include or not. 

My main thoughts on this are: 

1) I love this new approach. I appreciate all the work that was done on this. Would love to see this merged in. 

2) Once this is merged in, I can close the following Issues which I opened based on the previous approach we were using in check_code.sh / validate_docstrings.py 


> [DOC: fix GL08 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57443)
> [DOC: fix PR01 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57438)
> [DOC: fix PR07 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57420)
> [DOC: fix SA01 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57417)
> [DOC: fix RT03 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57416)
> [DOC: fix PR02 errors in docstrings](https://github.com/pandas-dev/pandas/issues/57111)

3) After closing the above issues, I can open a new issue to address fixing the docstrings that follows this new approach

4) And finally, there seems to be one cryptic failing CI check, [ASAN / UBSAN](https://github.com/pandas-dev/pandas/actions/runs/8310857572/job/22743812047?pr=57826#logs) - would like to see this resolved and all green on the CI, but as the logs got deleted, it's hard to tell if this is related to this PR or some outside issue. 



Thanks!

> * And finally, there seems to be one cryptic failing CI check, [ASAN / UBSAN](https://github.com/pandas-dev/pandas/actions/runs/8310857572/job/22743812047?pr=57826#logs) - would like to see this resolved and all green on the CI, but as the logs got deleted, it's hard to tell if this is related to this PR or some outside issue.

It seems that this test is currently failing on this and [many other PRs](https://github.com/pandas-dev/pandas/actions/runs/8307926216/job/22737627560?pr=57864), unit tests are failing on main as well.

I'd be happy to see this merged since resolving the conflicts with ongoing doc fix PRs is a hassle. Let me know if there is anything else blocking this.

Thanks again for all the great feedback and the welcoming atmosphere :)
Awesome! Lgtm 🙂
Hello, this is a great improvement! LGTM too.
Amazing job @dontgoto.

I see the `code_checks` job takes 20 minutes instead of 40 after this, and I think this will help a lot with the efforts to fix errors in docstrings.