8000 Support for non-default dataclass fields by waszil · Pull Request #116 · astropy/sphinx-automodapi · GitHub
[go: up one dir, main page]

Skip to content

Support for non-default dataclass fields #116

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

Closed
wants to merge 2 commits into from

Conversation

waszil
Copy link
@waszil waszil commented Sep 23, 2020

Hi, this PR tries to fix this issue: #115.
Used sphinx version: 3.2.1
Python version: 3.8.5

Fix #115

Base automatically changed from master to main March 9, 2021 20:00
@simaoafonso-pwt
Copy link
Contributor

This is small change, any reason why this is not merged?

@bsipocz
Copy link
Member
bsipocz commented Apr 13, 2023

This looks all fine, but needs a rebase and some minor renames/etc. I'll go ahead and do it, and see how CI reacts.

@bsipocz bsipocz force-pushed the dataclass-nondefault-fields branch from c83e14f to 91f6fc5 Compare April 13, 2023 23:42
@codecov
Copy link
codecov bot commented Apr 13, 2023

Codecov Report

Attention: Patch coverage is 68.96552% with 9 lines in your changes missing coverage. Please review.

Project coverage is 89.38%. Comparing base (8b2dbcf) to head (91f6fc5).
Report is 102 commits behind head on main.

Files with missing lines Patch % Lines
sphinx_automodapi/automodsumm.py 20.00% 8 Missing ⚠️
sphinx_automodapi/autodoc_enhancements.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   89.83%   89.38%   -0.45%     
==========================================
  Files          27       27              
  Lines        1141     1168      +27     
==========================================
+ Hits         1025     1044      +19     
- Misses        116      124       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member
@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments, and that this will need a changelog entry. Given the age of the PR I may just go ahead and do the changes to ensure this gets in.

Comment on lines +67 to +75
if dataclasses.is_dataclass(obj):
if attr in obj.__dataclass_fields__:
return obj.__dataclass_fields__[attr].name
else:
# raise original AttributeError
raise e
else:
# raise original AttributeError
raise e
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here could be simplified a bit and all put into one conditional and a raise outside of it.

return getattr(obj, attr, *defargs)
try:
return getattr(obj, attr, *defargs)
except AttributeError as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extreme nitpick: it's best to avoid single-letter variables and use something descriptive, even when the single letter feels trivial.

@lpsinger
Copy link
Contributor

This PR looked stale. I addressed the comments and made some other minor improvements in #208.

@pllim
Copy link
Member
pllim commented Apr 28, 2025

Superseded by #208

@pllim pllim closed this Apr 28, 2025
bsipocz added a commit that referenced this pull request Apr 28, 2025
Support for non-default dataclass fields (continuation of stale PR, #116)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for typing and dataclass fields
5 participants
0