Skip to content

implement a pydantic-settings-based Settings base class#16

Merged
ilia-kats merged 18 commits intomainfrom
settings
Apr 24, 2026
Merged

implement a pydantic-settings-based Settings base class#16
ilia-kats merged 18 commits intomainfrom
settings

Conversation

@ilia-kats
Copy link
Copy Markdown
Collaborator

The boilerplate is heavily inspired by AnnData's settings.

Forces validation on assignment, even if a subclass tries to disable it.

Unfortunately, it turns out that Pydantic doesn't really work with Sphinx that well (all the attributes and methods from BaseModel are added to the docs). I've tried sphinxcontrib-pydantic, but that has at least two issues that make it not work here. For now, I work around it by using a very minimal autosummary template for the Settings class.

This only implements the first part of #8. The second part (settings that apply to all scverse packages) needs some more thinking. One thing I could imagine is having a ScverseSettings class with global settings, and a sentinel type GLOBAL_DEFAULT. Each package's settings class can then add fields from the global settings and have them default to GLOBAL_DEFAULT. Customizing __getattribute__ in the base class, we could then return either the global setting, or the proper value if set by the user. This would allow users to override the globals for individual packages as needed. I'm still not sure about how to set the global settings, though. Should this be another class in scverse-misc (which would either require users to import it directly or other pakcages to re-export it)? Do we make another package scverse-settings?

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 98.87640% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 98.03%. Comparing base (df76cec) to head (6b72ccb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/scverse_misc/_settings.py 98.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   97.39%   98.03%   +0.64%     
==========================================
  Files           3        5       +2     
  Lines         115      204      +89     
==========================================
+ Hits          112      200      +88     
- Misses          3        4       +1     
Files with missing lines Coverage Δ
src/scverse_misc/__init__.py 100.00% <100.00%> (ø)
src/scverse_misc/_utils.py 100.00% <100.00%> (ø)
src/scverse_misc/_settings.py 98.50% <98.50%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ilia-kats
Copy link
Copy Markdown
Collaborator Author

the failing CI looks like some issue with codecov, unrelated to the changes here.

Comment thread src/scverse_misc/__init__.py Outdated
Comment thread src/scverse_misc/__init__.py
flying-sheep and others added 4 commits April 23, 2026 14:10
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

Looking good!

I fixed the types in 6a65ee7 – the biggest change is that I found the use of the implicit __class__ variable weird. It’s identical to subcls right? So I just renamed subcls to cls (as is convention) and made _get_packagename a classmethod instead of a staticmethod taking a class.

Comment thread src/scverse_misc/_settings.py
Comment thread src/scverse_misc/_settings.py
Comment thread src/scverse_misc/_settings.py Outdated
Comment thread tests/test_settings.py
@ilia-kats
Copy link
Copy Markdown
Collaborator Author

I found the use of the implicit class variable weird. It’s identical to subcls right? So I just renamed subcls to cls (as is convention) and made _get_packagename a classmethod instead of a staticmethod taking a class.

__class__ is a special compiler-generated binding that always refers to the class where it's being used. I spefically used it here to call our _get_packagename method and not let subclasses override it.

@ilia-kats ilia-kats requested a review from flying-sheep April 23, 2026 14:30
Copy link
Copy Markdown

@ilan-gold ilan-gold left a comment

Choose a reason for hiding this comment

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

Nice!

Comment thread src/scverse_misc/_settings.py Outdated
Comment thread src/scverse_misc/_settings.py
flying-sheep and others added 2 commits April 23, 2026 17:47
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for this!

I’d prefer no default for docstring_style, otherwise I think this looks perfect!

package_name = package_name[:dotidx]
return package_name

def __init_subclass__(subcls, *, exported_object_name: str, docstring_style: Literal["google", "numpy"] = "google"):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer no default here so this is explicit.

Suggested change
def __init_subclass__(subcls, *, exported_object_name: str, docstring_style: Literal["google", "numpy"] = "google"):
def __init_subclass__(subcls, *, exported_object_name: str, docstring_style: Literal["google", "numpy"]):

@ilia-kats ilia-kats merged commit 7960635 into main Apr 24, 2026
9 checks passed
@ilia-kats ilia-kats deleted the settings branch April 24, 2026 08:46
@flying-sheep flying-sheep linked an issue Apr 24, 2026 that may be closed by this pull request
@flying-sheep
Copy link
Copy Markdown
Member

@ilia-kats #16 (comment)

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.

settings class

4 participants