mkdocstrings / pytkdocs

Load Python objects documentation.
https://mkdocstrings.github.io/pytkdocs
ISC License
50 stars 32 forks source link

feat: Add `trim_doctest_flag` to google and numpy parsers #134

Closed thatlittleboy closed 2 years ago

thatlittleboy commented 2 years ago

Related to: https://github.com/mkdocstrings/mkdocstrings/issues/386

I've decided to start the work on the above issue starting from the legacy parser. Please take a look and see if I'm going in the right direction or if I've missed anything crucial.

Key PR pointers

Addendum

pawamoy commented 2 years ago

You can rebase on main branch to fix Windows builds.

Only implemented for Google and Numpy for now. I'm not sure how to do it for Rst, it doesn't seem like Rst is parsing examples explicitly (or at all?). If there is a way to implement for Rst, I would be grateful if you could nudge me in the right direction. I'll try to incorporate it into this PR as well. (since our pyjanitor project is using the rst / legacy-python combination sweat_smile

Indeed, the RST parser does not even parse code examples. I will not ask you to add it, but if you want to do so, be my guest. Also, are you really using reStructuredText in your pyjanitor docstrings? I have to say that I strongly recommend switching to a style more akin to Markdown (google or even numpydoc): after all, you are generating docs with MkDocs, which uses Markdown exclusively :confused: Maybe you have this planned though, in that case, just ignore my comment :slightly_smiling_face:

As to pointing you in the right direction if you want to try and implement code example parsing in the RST parser: I won't be of any help unfortunately. The RST parser was implemented by @plannigan.

thatlittleboy commented 2 years ago

@pawamoy

  1. Rebase on main? I don't see a main branch on the pytkdocs repo 😅
  2. Got it, thanks! Yeah the story with pyjanitor is that they switched over from Sphinx to mkdocs about last year (before I was around), I just supposed they stuck with rst for ease of transition. I think long-term the direction should be to switch over to the new parser, or at the very least switching over to google, as you mentioned. No worries then, this PR will just be targeted towards Google and Numpy. I think there'll be more value in porting this change over to griffe.
pawamoy commented 2 years ago

Arf, I thought I was on another repo, my bad! The main branch is master here indeed :slightly_smiling_face:

I checked your docstrings: you're not really using RST I think, I just mislead myself by naming the parser "RST", when the :param name: syntax is just Sphinx-style I think. This is why I renamed this parser to sphinx in Griffe, less confusing.

pawamoy commented 2 years ago

The type check failed but I will fix it just after merging your PR if you don't mind.

thatlittleboy commented 2 years ago

Sure, thanks!