Skip to content

fix: use the real class name in the autosummary class template - #71

Merged
ilia-kats merged 3 commits into
mainfrom
ik/class-template
Oct 21, 2026
Merged

ilia-kats merged 3 commits into
mainfrom
ik/class-template

Conversation

@ilia-kats

Copy link
Copy Markdown
Contributor

This breaks handling of module aliases and is unnecessary: The proper indentation of the items with respect to the autoclass directive tells Sphinx where this belongs.

Compare for example this new build log using scverse-doc to this older one that was still using scverse-misc.

This breaks handling of module aliases and is unnecessary: The proper
indentation of the items with respect to the autoclass directive tells
Sphinx where this belongs.
@read-the-docs-community

read-the-docs-community Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 scverse-doc | 🛠️ Build #35296174 | 📁 Comparing 47ea07e against latest (34d4574)

  🔍 Preview build  

1 file changed
± changelog.html

@ilia-kats

ilia-kats commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor Author

I think the root cause of the failing test is AttributeError: module 'apimod' has no attribute '__path__', which is the case in the test setup because it's literally just a Python file lying around. In real packages, that will never be the case, modules will always be part of a package, so I think the apimod test is not necessary.

@flying-sheep

flying-sheep commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

I don’t think the failure is completely bogus, as you’re essentially reverting #47, which made the following documentable:

class Test:
    test: int
    """An attribute."""

So let’s not remove the test! Let’s:

  1. fix the test until it shows the actual failure that I’m pretty sure this change causes /edit: no fix needed, it was correct all along
  2. add a test for your use case
  3. Fix things so both use cases work

@flying-sheep

flying-sheep commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Yeah, the test was actually correct. I added a new one for your use case and let Claude figure out how to get to the real class name (which turned out to be in fullname even for aliases)

This might still break if you have a nested class (__qualname__ has a dot in it) but let’s cross that bridge once we get to it.

Can you test if that works for the project?

@codecov

codecov Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (34d4574) to head (47ea07e).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #71   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          650       650           
=========================================
  Hits           650       650           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@flying-sheep flying-sheep changed the title fix: don't use the object's full name in the autosummary class template fix: use the real class name in the autosummary class template Oct 21, 2026
@ilia-kats

Copy link
Copy Markdown
Contributor Author

Thanks, I didn't realize that the instance variable was the problem. It seems like a Sphinx (usability) issue that class variables work with just the attribute name and instance variables require the class name to be prefixed. 1

This seems to work for my project, merging.

Footnotes

  1. (for future reference: If the instance var does not have a class prefix, this line raises "not enough values to unpack". Shouldn't be too difficult to fix.). ↩

@ilia-kats
ilia-kats merged commit 1130827 into main Oct 21, 2026
16 checks passed
@ilia-kats
ilia-kats deleted the ik/class-template branch October 21, 2026 14:39
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

Successfully merging this pull request may close these issues.

2 participants