Raise instead of silently dropping custom codec

[TomNicholas]

From discussion on #766:

We still likely have bugs because a user can define their own codec that doesn't subclass from Zarr python. In that case, their custom codec will be dropped

We should prefer raising instead of silently dropping a codec, because with a missing codec the user will never have their data deserialized correctly.

cc @maxrjones

[TomNicholas]

Notes to self:

• Custom codecs are actually fine, but only so long as they inherit from one of ArrayArrayCodec, ArrayBytesCodec, or BytesBytesCodec.
• The reason being that if they don't inherit from one of those, we don't know where they are supposed to go in the codec ordering in

VirtualiZarr/virtualizarr/codecs.py

Lines 49 to 63 in f3149d6

	def extract_codecs(
	codecs: CodecPipeline,
	) -> DeconstructedCodecPipeline:
	"""Extracts various codec types."""
	arrayarray_codecs: tuple[ArrayArrayCodec, ...] = ()
	arraybytes_codec: ArrayBytesCodec | None = None
	bytesbytes_codecs: tuple[BytesBytesCodec, ...] = ()
	for codec in codecs:
	if isinstance(codec, ArrayArrayCodec):
	arrayarray_codecs += (codec,)
	if isinstance(codec, ArrayBytesCodec):
	arraybytes_codec = codec
	if isinstance(codec, BytesBytesCodec):
	bytesbytes_codecs += (codec,)
	return (arrayarray_codecs, arraybytes_codec, bytesbytes_codecs)

• So the fix for this issue should check that there are no codecs left that are not captured by that logic.
• The test should pass a "codec" class that does not inherit from one of the zarr-python base classes, and assert that an error is raised.

[TomNicholas]

I'm getting quite confused by which, if any, of extract_codecs, convert_to_codec_pipeline, and create_v3_array_metadata should be public.

[TomNicholas]

#775 closes this, at least for code that uses extract_codecs. But I'm not sure how much user code actually does call extract_codecs, for the reason above. IIUC also our kerchunk writer never calls extract_codecs.

[maxrjones]

FWIW I would love it if we could get rid of that entire module. See NASA-IMPACT/veda-odd#150 for some additional notes. clearly I was overoptimistic that our dev docs suffice 😓

Also, (as I've probably mentioned elsewhere) I have been intentionally avoiding spending too much time on codecs in Virtualizarr because there's some redundancy with upstream efforts in Zarr. Just worth being aware that zarr-developers/zarr-python#3162 may make some work obsolete, because that may bring us to a point where it's possible to use the top-level API in most cases rather than manipulating ArrayMetadata (xref

VirtualiZarr/virtualizarr/writers/icechunk.py

Line 341 in a8e0ac7

# TODO: Should codecs be an argument to zarr's AsyncrGroup.create_array?

). If @d-v-b agrees that we could get to that point, it might be most efficient to support his codec simplification plan rather than continuing patching up these workarounds.