-
Notifications
You must be signed in to change notification settings - Fork 164
(feat): composable API for xarray dataset #1997
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
base: main
Are you sure you want to change the base?
Conversation
…to ig/setting_on_dataset
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1997 +/- ##
==========================================
- Coverage 87.14% 85.21% -1.94%
==========================================
Files 46 46
Lines 6986 7033 +47
==========================================
- Hits 6088 5993 -95
- Misses 898 1040 +142
🚀 New features to boost your workflow:
|
…to ig/setting_on_dataset
…anndata into ig/composition_over_inheritance
Co-authored-by: Philipp A. <[email protected]>
Co-authored-by: Philipp A. <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a bunch of documentation issues, otherwise it looks fine!
please go through every single documented item and make sure the docs look right, there is broken syntax in some.
docs/conf.py
Outdated
nitpick_ignore_regex = [ | ||
# See https://github.com/python/cpython/blob/7775d93e2d142ab66c7d8fa7cff068b2522a37b0/Lib/_collections_abc.py#L792-L817 | ||
# for why these are necessary. | ||
# The docstrings are just normal strings and thus misinterpreted. | ||
# TODO: revert once this is resolved in python. | ||
("py:class", ".*d defaults to None.*"), | ||
("py:class", "a.*object providing.*a view on D.*"), | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that these strings end up in our docs at all is the problem, so instead of ignoring errors arising from that, please prevent the source of the issue
- either by not creating documentation for inherited methods (e.g. like this), and instead linking to
Mapping
and explaining that we inherit from it (e.g. setting"show-inheritance": True
inautodoc_default_options
) - or by overriding them with our own docstrings
nitpick_ignore_regex = [ | |
# See https://github.com/python/cpython/blob/7775d93e2d142ab66c7d8fa7cff068b2522a37b0/Lib/_collections_abc.py#L792-L817 | |
# for why these are necessary. | |
# The docstrings are just normal strings and thus misinterpreted. | |
# TODO: revert once this is resolved in python. | |
("py:class", ".*d defaults to None.*"), | |
("py:class", "a.*object providing.*a view on D.*"), | |
] |
…anndata into ig/composition_over_inheritance
Following up on a conversation with Luca, a composable API is like both more maintainable and more explainable. It also represents a concrete step towards something like #1111. This PR represents a first step as it forces us to more clearly enumerate the methods needed for internal compatibility that were hidden by subclassing from
Dataset
i.e.,copy
andreindex