Skip to content

Initialize custom backends in open_dataset - #5684

Merged
max-sixty merged 9 commits into
pydata:mainfrom
Illviljan:Illviljan-custom_backends_should_init
Oct 9, 2021
Merged

Initialize custom backends in open_dataset#5684
max-sixty merged 9 commits into
pydata:mainfrom
Illviljan:Illviljan-custom_backends_should_init

Conversation

@Illviljan

@IllviljanIllviljan commented Aug 7, 2021

Copy link
Copy Markdown
Contributor

The backend classes are initialized in the build_engine function:

return {name: backend() forname, backendinbackend_entrypoints.items()}

This wasn't the case for custom backends:

backend=engine

This PR initializes the engine, fixes the incorrect signature in the test (#5033) and reverts the doc changes done in #5532.

  • Tests added
  • Passes pre-commit run --all-files
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@github-actions

github-actionsBot commented Aug 7, 2021

Copy link
Copy Markdown
Contributor

Unit Test Results

6 files 6 suites 56m 48s ⏱️
16 226 tests 14 491 ✔️ 1 735 💤 0
90 552 runs 82 368 ✔️ 8 184 💤 0

Results for commit a4d12c1.

♻️ This comment has been updated with latest results.

@IllviljanIllviljan changed the title Initialize custom backendsInitialize custom backends in open_datasetAug 7, 2021
@Illviljan

Copy link
Copy Markdown
ContributorAuthor

I noticed this issue because my local mypy version was complaining about:
xrtools.py:750: error: Signature of "open_dataset" incompatible with supertype "BackendEntrypoint"

Anyone have any ideas why the CI version doesn't complain about this?

@jhamman
jhamman requested a review from aurghsAugust 9, 2021 01:31
@Illviljan

Illviljan commented Aug 10, 2021

Copy link
Copy Markdown
ContributorAuthor

I noticed this issue because my local mypy version was complaining about:
xrtools.py:750: error: Signature of "open_dataset" incompatible with supertype "BackendEntrypoint"

Anyone have any ideas why the CI version doesn't complain about this?

The original test function did not have any typing defined. For mypy to do any signature checking either of the inputs or outputs needs to be typed.

This will throw the supertype error, uncomment self to get it passing:

importnumpyasnpimportxarrayasxrfromxarray.backendsimportBackendEntrypointexpected=xr.Dataset(
dict(a=2*np.arange(5)), coords=dict(x=("x", np.arange(5), dict(units="s")))
)
classCustomBackend(BackendEntrypoint):
defopen_dataset(
# self,filename_or_obj,
drop_variables=None,
**kwargs,
) ->xr.Dataset:
returnexpected.copy(deep=True)

@keewiskeewis added the plan to merge Final call for comments label Sep 20, 2021
@keewis

Copy link
Copy Markdown
Collaborator

This looks ready to me, and I think we should merge soon.

@max-sixty
max-sixty merged commit fd42449 into pydata:mainOct 9, 2021
snowman2 pushed a commit to snowman2/xarray that referenced this pull request Feb 9, 2022
* Test using correct signature
* Initialize the class like in build_engine
* Update whats-new.rst
* Update whats-new.rst
* Add back self to classes
* Add some typing to the test, mypy should fail
* mypy failed as expected, should not now
Co-authored-by: keewis <keewis@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plan to mergeFinal call for comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Illviljan@keewis@max-sixty