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

Trim documentation for pre-PEP8 aliases #411

Closed
djpohly opened this issue Jun 10, 2022 · 9 comments
Closed

Trim documentation for pre-PEP8 aliases #411

djpohly opened this issue Jun 10, 2022 · 9 comments
Assignees

Comments

@djpohly
Copy link
Contributor

djpohly commented Jun 10, 2022

Pre-PEP8 aliases are currently defined as references to the new objects, which means autodoc sees two names with the same docstring and generates two copies of the documentation for each one. This duplicate information accounts for over 15% (!) of the page space in the pyparsing API reference.

I decided to file an issue before making a PR to find out what the devs' preference would be (plus, I don't know how long the deprecated aliases are meant to stay around). The documentation for an alias could be:

  • a copy of the full documentation (as it is now)
  • a deprecation note with a link to the updated name
  • a deprecation note and link, followed by a copy of the full documentation
  • hidden altogether

I have a pretty good idea how each option could be implemented without copy-pasting documentation, so I'm curious which would be preferred. (We might even be able to hook autodoc-process-docstring to trim the Sphinx output down while keeping the full documentation in __doc__ for the REPL.)

@ptmcg
Copy link
Member

ptmcg commented Jun 16, 2022

Thanks for raising this - I'm not really happy to see doc duplicated for names like asDict, asList, etc. when as_dict and as_list are the preferred names now.

My initial thought is that a deprecation note and link to the new PEP8 name would make the most sense. I'll do some experimenting also in this area.

@ptmcg
Copy link
Member

ptmcg commented Jun 16, 2022

Making significant progress on this, including some nicer decorators and adding DeprecationWarnings

ptmcg added a commit that referenced this issue Jun 16, 2022
…d of duplicating actual function doc) - issue #411
@ptmcg
Copy link
Member

ptmcg commented Jun 16, 2022

I've held off on the DeprecationWarnings for now, I'll enable them in 3.1, to be removed in some later version, maybe not even until 4.0.

@ptmcg
Copy link
Member

ptmcg commented Jun 16, 2022

Looking at the latest docs, it seems I've still left in a bunch of duplicate doc strings (such as setDebug, setName, etc. on ParserElement and subclasses). If you want, open a PR to fix them using the replaces_prePEP8_function decorator.

@djpohly
Copy link
Contributor Author

djpohly commented Jun 16, 2022

In case there's anything that might be useful there, I did some work on this too on the pre-pep8-in-docs branch. I tried to make sure the wrappers still work with static type-checkers, and maybe went a little overboard in trying not to introduce any additional function-call overhead (probably should have done some benchmarks first, but it was fun so hey).

@ptmcg
Copy link
Member

ptmcg commented Jun 16, 2022

I am less concerned about extra overhead at parser creation time (typically a one-time, upfront cost) than I am about overhead during the actual parse (which is where I get the most complaints about pyparsing being slow).

I'll look at that branch later today, thanks!

@ptmcg
Copy link
Member

ptmcg commented Jun 17, 2022

And all you wanted to do was clean up some docstrings. Thanks again for the help and PRs!

djpohly added a commit to djpohly/pyparsing that referenced this issue Jun 18, 2022
@djpohly
Copy link
Contributor Author

djpohly commented Jun 18, 2022

Well... after learning a bit more about Sphinx today, I might feel a little sheepish. I think a lot of the "creative" approach here might have been avoided just by pointing Sphinx at the real modules (pyparsing.core, pyparsing.results, and so on) rather than at the root pyparsing module. Sphinx can pick up a docstring in a source file from where an alias is created, but since the string isn't actually stored with the variable itself, it's not accessible from the other side of the import in __init__.py.

See draft PR #415 for what I'm talking about.

@ptmcg
Copy link
Member

ptmcg commented Apr 19, 2023

I'm going to close this, I think we've done our due diligence on updating the deprecated names, and are positioned well to start emitting DeprecationWarnings in the next minor release. Thanks again!

@ptmcg ptmcg closed this as completed Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants