Mesh fetching fails for some combinations

[dickscheid]

Using siibra version 1.0a05, when trying to fetch one of the cortical layer meshes, I fail to do so from the map object:

import siibra
m = siibra.get_map('cortical layers', space='bigbrain')
# choosing the mesh provider explicitly
p = m.volumes[0]._providers['neuroglancer/precompmesh']
# fetch from mesh provider - works
p.fetch(fragment='left', label='1')
# Have the initial ojbect select the provider - fails
m.fetch(format='mesh', fragment='left', label='1') 

The response is:

File ~/src/siibra-python/siibra/volumes/parcellationmap.py:422, in Map.fetch(self, region_or_index, index, region, **kwargs)
    418     raise IndexError(
    419         f"{self} provides {len(self)} mapped volumes, but #{mapindex.volume} was requested."
    420     )
    421 try:
--> 422     result = self.volumes[mapindex.volume or 0].fetch(
    423         fragment=mapindex.fragment, label=mapindex.label, **kwargs
    424     )
    425 except requests.SiibraHttpRequestError as e:
    426     print(str(e))

TypeError: siibra.volumes.volume.Volume.fetch() got multiple values for keyword argument 'label'

[AhmetNSimsek]

The map.fetch() does not expose label kwarg by design (not sure exact version but during refactoring for 0.4a). This has to be supplied via a MapIndex or a region so that MapIndex can be decoded internally. This PR recognizes the potential legacy users and works to reintroduce the ability to use label as kwarg. But I am not sure this should be the case.

[dickscheid]

Maybe it would help to properly document the current behavior. The above example is a good guide for this, since I wasn't able to infer any valid fetching call without digging under the hood. Can we have fetch() outputting valid variants when failing? Seeing a table of what could be fetched from the map object would be helpful, in terms of type, fragment, ...

[dickscheid]

In fact I think the behavior is inconsistent currently. Looking at the code snippet above,

• p = m.volumes[0]._providers['neuroglancer/precompmesh'] explicitly selects one of the volume providers. Implicit selection of the provider is meant to be steered by passingformat='neuroglancer/precompmesh' to m.fetch(), so using this parameter I expect the Map object effectively chooses this provider for me (and I think it does so indeed).
• unused kwargs should be passed on to the selected provider, so adding label='1' to m.fetch() should be carried through to the selected provider.
• Thus, if p.fetch(format='mesh', fragment='left', label='1') works, one would expect m.fetch(format='neuroglancer/precompmesh', format='mesh', fragment='left', label='1') to do the same.

So isn't this just a bug that the implementation doesn't realize the label kwarg was already there?+

[AhmetNSimsek]

IMO, the docstring already clearly states the kwargs.

import siibra
from siibra.commons import MapIndex
m = siibra.get_map('cortical layers', space='bigbrain')
m.fetch(MapIndex(fragment='left', label=1), format='mesh')

This is the current usage if one would like to use labels or volumes, ie to specify the MapIndex.

[dickscheid]

The map.fetch() does not expose label kwarg by design (not sure exact version but during refactoring for 0.4a). This has to be supplied via a MapIndex or a region so that MapIndex can be decoded internally. This PR recognizes the potential legacy users and works to reintroduce the ability to use label as kwarg. But I am not sure this should be the case.

I think we need not handle 'label' explicitly. I would expect any kwargs are passed through to the receipient (here: neuroglancer pre comp mesh provider) if unused by the caller (here: Map).

I will check though if this is unnecessary when passing a region, and report here. I agree that users typically do not deal with the label but with region names.

[AhmetNSimsek]

The map.fetch() does not expose label kwarg by design (not sure exact version but during refactoring for 0.4a). This has to be supplied via a MapIndex or a region so that MapIndex can be decoded internally. This PR recognizes the potential legacy users and works to reintroduce the ability to use label as kwarg. But I am not sure this should be the case.

I think we need not handle 'label' explicitly. I would expect any kwargs are passed through to the receipient (here: neuroglancer pre comp mesh provider) if unused by the caller (here: Map).

I will check though if this is unnecessary when passing a region, and report here. I agree that users typically do not deal with the label but with region names.

Before investing more time, have you seen the PR I mentioned? I think I have a solution already.

[dickscheid]

So, the following does work:

import siibra
layer maps = siibra.get_map('cortical layers', space='bigbrain')
l4mesh = layermaps.fetch(region="4 left", format='mesh')

This is probably the intended use.