astropy / sphinx-automodapi

Sphinx extension for generating API documentation
https://sphinx-automodapi.readthedocs.io
BSD 3-Clause "New" or "Revised" License
65 stars 45 forks source link

Add sort option to automodsumm #182

Closed nstarman closed 7 months ago

nstarman commented 10 months ago

Ping @mhvk

nstarman commented 10 months ago

Maybe we should change the option from sort to alphabetize? OTOH. @mhvk had an idea about grouping imports my module or some other logical grouping. Should sort not be a boolean flag, but an enumeration? Or would that be a new flag, groupby? E.g. then :groupby: + :sort: would group the objects and sort within each group.

codecov[bot] commented 10 months ago

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 90.28%. Comparing base (5ab68d0) to head (2b01690). Report is 4 commits behind head on main.

Files Patch % Lines
sphinx_automodapi/automodapi.py 60.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #182 +/- ## ========================================== + Coverage 90.24% 90.28% +0.03% ========================================== Files 28 28 Lines 1179 1204 +25 ========================================== + Hits 1064 1087 +23 - Misses 115 117 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mhvk commented 10 months ago

sort in python is pretty unambiguous, so I would just stick with it.

We do need to add a test...

pllim commented 9 months ago

@nstarman , are you still interested in pursuing this?

nstarman commented 9 months ago

Yes. Thanks for putting it back on my radar.

nstarman commented 9 months ago

Hm. Not sure why the test is failing so early. I copied it from a test above!

pllim commented 9 months ago

I cannot tell why it is failing from the log. You might need to run this test locally and debug.

nstarman commented 7 months ago

I would welcome help on the test suite for this PR. The PR works when tested on Astropy, I just can't wrangle this library's test suite.

mhvk commented 7 months ago

@nstarman - I think I found the problem: you missed how :sort: is also dealt with in writing summary lines. And that showed a problem with the test too... See https://github.com/nstarman/sphinx-automodapi/pull/1 (as I'm not a maintainer, I cannot push to your repository...)

nstarman commented 7 months ago

Thanks @mhvk! I just cherry-picked your commit from your branch https://github.com/mhvk/sphinx-automodapi/tree/sort-option

nstarman commented 7 months ago

Rebased to fix conflict.

nstarman commented 7 months ago

Ping @eteq, I think this is now ready for review!

nstarman commented 7 months ago

Done! Had to google how VSCode can do saving w/out formatting (I have mine set up to run code quality tools on save).

bsipocz commented 7 months ago

Do you need an immediate release?

I wonder whether we could clean out 1 or 2 more of the remaining PRs, so we don't end up with single PR releases :) Most are adding functionality but don't include tests, so while it's a bit of work to wrap them up I don't expect it to be too complicated.