[24.1] Fix h5ad metadata

[nilchia]

This PR fixes h5ad metadata problem.

The metadata of h5ad is not shown correctly in the history:

This little change fixes that :)

[bernt-matthias]

Wondering if we should add a test for this, analogous to https://github.com/galaxyproject/galaxy/blob/dev/test/unit/data/datatypes/test_qiime2.py

[dannon]

@bernt-matthias Yes, for sure. I add a visualization test in #18552 that inadvertently exercises this as well, and I wonder if this explains that test failure that I haven't got around to fixing up yet :)

Edit: Followed up on that test failure -- that was due to the test suite not loading the h5 datatype at all, so unrelated, though I'm still +1 on a datatype test here.

[nilchia]

Dear all,
I made a test file (I hope that's correct :) ).
I have an anndata as a test file but I don't know which directory to put that.

[bgruening]

How large is it? If its too large put it into https://github.com/galaxyproject/galaxy-test-data

[nilchia]

It is 74.0 kB

[bgruening]

Then have a look where qiime2.qza is stored and put your file next to it :)

[nilchia]

Hi @bgruening @davelopez @bernt-matthias @dannon,
Thanks for your review. Is the test good now?

[bgruening]

Suggested change

	if not dataset.file_name_ is None:
	if dataset.file_name_ is not None:

[mvdbeek]

Why did you make that change, this looks quite wrong to me.

[nilchia]

it gives this error:

    def set_peek(self, dataset: DatasetProtocol, **kwd) -> None:
>       if dataset.dataset.purged:
E       AttributeError: 'NoneType' object has no attribute 'purged'

the MockDataset class does not have purge attribute:

class MockDataset:
    def __init__(self, id):
        self.id = id
        self.metadata = MockMetadata()
        self.dataset = None
        self.file_name_: Optional[str] = None

    def get_file_name(self, sync_cache=True):
        return self.file_name_

    def set_file_name(self, file_name):
        self.file_name_ = file_name

    def has_data(self):
        return True

    def get_size(self):
        return self.dataset and os.path.getsize(self.dataset.get_file_name())
        ```
        

[mvdbeek]

Please don't adjust code for unit tests.

[nilchia]

Could you please give me a hint of what I should do?
That class does not have "purged" attribute. am I right?

[bgruening]

What was wrong with the dataset size?

[nilchia]

It gives this error:

lib/galaxy/datatypes/binary.py:1599: in set_peek
    dataset.blurb = f"Anndata file ({nice_size(dataset.get_size())})"
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

size = None

    def nice_size(size: Union[float, int, str, Decimal]) -> str:
        """
        Returns a readably formatted string with the size
    
        >>> nice_size(100)
        '100 bytes'
        >>> nice_size(10000)
        '9.8 KB'
        >>> nice_size(1000000)
        '976.6 KB'
        >>> nice_size(100000000)
        '95.4 MB'
        """
        try:
>           size = float(size)
E           TypeError: float() argument must be a string or a number, not 'NoneType'

Maybe sth is wrong with nice_size ?
it is in init.py

[mvdbeek]

Suggested change

	if dataset.file_name_ is not None:
	if not dataset.dataset.purged:

[mvdbeek]

This file is too large, can you create a smaller one please ?

[nilchia]

Is it fine now?

[mvdbeek]

Yes, thanks, we'll use that later for a different type of test.

[mvdbeek]

You've now tailored the code to the mock implementation and it will not actually work. If the core team decides we want to validate metadata attributes we can do this more systematically. I would be in favor of merging only the fix.

[nilchia]

This fix worked on my local galaxy with only changing layers = list(tmp.attrs) to layers = list(tmp.keys()) without other changes.

But for making a test, the other classes (like MockDataset) are problematic.

[mvdbeek]

You can add the required attributes to the mock class if you like, but the fix seems fine as is.

[mvdbeek]

Thanks a lot @nilchia!

[nilchia]

Thank you @mvdbeek!

[github-actions]

This PR was merged without a "kind/" label, please correct.