Backends entrypoints

[aurghs]

• It's an update of @jhamman pull request [Feature] Backend entrypoint #3166
• It uses entrypoints module to detect the installed engines. The detection is done at open_dataset function call and it is cached. It raises a warning in case of conflicts.
• Add a class for the backend interface BackendEtrypoint instead of a function.

Modified files:

• add plugins.py containing detect_engines function and BackendEtrypoint.

• dependencies file to add entrypoints.

• backend.init to add detect_engines

• apiv2.py and api.py do use detect_engines

• zarr.py, h5netcdf_.py, cfgrib.py to instatiate the BackendEtrypoint.

• Related to [Feature] Backend entrypoint #3166

• Tests added

• Passes isort . && black . && mypy . && flake8

[pep8speaks]

Hello @aurghs! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-12-10 09:10:08 UTC

[jhamman]

Thanks @aurghs for pushing this forward. I have a few small comments but think this is otherwise quite close. We should discuss (tomorrow I suppose) a plan for testing the entrypoint discovery logic and error handling.

Just to bring to the attention of the rest of @pydata/xarray, this does add a new, small dependency on the entrypoints package.

[aurghs]

• I have replaced entrypoints with pkg_resources. I can't see any drawback in this change, but only advantages, the main one is that we remove an external dependency.
• I have added the tests.
• I didn't remove the signature inspection, since we have already talked about it widely with @shoyer, and in the end, we decided to keep the inspection and add the checks on the signature to be sure that neither *args nor **kwargs will be used.

[keewis]

I have a few suggestions which I feel will improve the readability, but I'm not sure if that's objectively true for each of these.

Also, as a note to myself and to others trying to test this (I spent way too much time trying to figure that out): after changing the entrypoints in setup.cfg, the package has to be reinstalled for the changes to be visible (editable installs only allow changing the code without reinstalling).

[keewis]

for this sort of filtering and transformation I would usually rely on comprehensions:

Suggested change

	for d in list(decoders):
	if decode_cf is False and d in open_backend_dataset_parameters:
	decoders[d] = False
	if decoders[d] is None:
	decoders.pop(d)
	return decoders
	def choose_decoder(name):
	if decode_cf is False and name in open_backend_dataset_parameters:
	return False
	return decoder
	return {
	choose_decoder(name)
	for name, decoder in decoders.items()
	if decoder is not None
	}

but there might be a better (more readable) way to express the conditional transformation of decoder. Maybe use a local function with a descriptive name? Edit: I added the local function, but I'm still searching for a better name

[jhamman]

@aurghs - I think we're basically done here. Can you clean up the remaining failing tests and we'll get this merged?

[alexamici]

@keewis WRT your comments on list comprehensions we at B-Open prefer to use them only when they make the code one line and actually prefer explicit loops otherwise.

This is both because we find them more readable as code and because tracebacks as well are easier to parse when they point to the statement that raised.

[keewis]

no worries, as I said when posting these suggestions I do know that there are different opinions about comprehensions or generator expressions. I could have been more explicit about this, though, I meant to say that you may choose not to apply suggestions you do not agree with.