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

Always use __dict__ and ignore __slots__ on classes #169

Closed Nodd closed 11 months ago

Nodd commented 1 year ago

Fix #168

Nodd commented 1 year ago

The workflow failure seems unrelated to my modification.

codecov[bot] commented 1 year ago

Codecov Report

Merging #169 (6bbddc0) into main (ae69f55) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   89.83%   89.88%   +0.05%     
==========================================
  Files          27       27              
  Lines        1141     1137       -4     
==========================================
- Hits         1025     1022       -3     
+ Misses        116      115       -1     
Impacted Files Coverage Ξ”
sphinx_automodapi/automodsumm.py 85.84% <100.00%> (+0.13%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

pllim commented 1 year ago

This is related though.

sphinx_automodapi/automodsumm.py:86:1: F401 'abc' imported but unused
1

I'll have to ping @eteq , @jairideout , and @pkgw based on the blame on the affected function.

pllim commented 1 year ago

RTD failure is indeed unrelated. I don't know why it is pulling such old versions.

Nodd commented 1 year ago

This is related though.

sphinx_automodapi/automodsumm.py:86:1: F401 'abc' imported but unused
1

I fixed this, but there was another error : Reason: UndefinedError("'style' is undefined")

I'll have to ping @eteq , @jairideout , and @pkgw based on the blame on the affected function.

Yeah, I guess the added complexity was there for a reason ? But if include_base is True there is no special handling of __slots__ and everything works correctly in my case.

pllim commented 1 year ago

@Nodd , unfortunately I wasn't involved in the original design of the code, so I have no idea what it is trying to do, but hopefully one of the contributors could comment more. 🀞

I ~am trying to fix~ fixed RTD over at #170. If that works, I'll merge it and then you can rebase to pick up the fix. 🀞 🀞

Thanks for your contribution and for your patience!

Nodd commented 1 year ago

Looks like the rebase went well!

Nodd commented 1 year ago

OK, I'll do this as soon as I can find some time.

kylefawcett commented 11 months ago

I ran into this problem as well when attempting to automodapi a module containing some classes that extend pydantic's BaseModel which uses __slots__. I see that @Nodd has the fix in this PR but it appears to be waiting for unit testing. To help this along, I'd like to contribute some test I've implemented here.

@Nodd can cherry pick the commit with the tests, or I can create a new PR. No preference on my end.

pllim commented 11 months ago

Thanks for your interested in this PR! Since it has been 6 months, if a new PR could be made with cherry picking the original commits here + new stuff needed to push it over the finish line, that would be great! πŸ™‡β€β™€οΈ

Nodd commented 11 months ago

Sorry, I never found the time to finish this. @kylefawcett you can create a new PR, I'll be very happy if you manage to get it done :smiley:

kylefawcett commented 11 months ago

@pllim -- Will do. @Nodd -- sounds good. I've cherry picked the commits with your fix as is so you'll still be in the commit history. I added my test cases on top of that in a new branch which will be submitted as a new PR soon.

pllim commented 11 months ago

Thanks, all! Superseded by #181