Iterating over a vision dataset doesn't decode the images
State: closed | Author: lhoestq (MEMBER)
Labels: name: bug

name: vision

## Describe the bug

If I load `mnist` and I iterate over the dataset, the images are not decoded, and the dictionary with the bytes is returned.

## Steps to reproduce the bug
```python
from datasets import load_dataset
import PIL

mnist = load_dataset("mnist", split="train")

first_image = mnist[0]["image"]
assert isinstance(first_image, PIL.PngImagePlugin.PngImageFile)  # passes
first_image = next(iter(mnist))["image"]
assert isinstance(first_image, PIL.PngImagePlugin.PngImageFile)  # fails
```

## Expected results

The image should be decoded, as a PIL Image

## Actual results

We get a dictionary
```
{'bytes': b'\x89PNG\r\n\x1a\n\x00..., 'path': None}
```

## Environment info

- `datasets` version: 1.17.1.dev0
- Platform: Darwin-20.6.0-x86_64-i386-64bit
- Python version: 3.7.2
- PyArrow version: 6.0.0

The bug also exists in 1.17.0

## Investigation

I think the issue is that decoding is disabled in `__iter__`:

https://github.com/huggingface/datasets/blob/dfe5b73387c5e27de6a16b0caeb39d3b9ded66d6/src/datasets/arrow_dataset.py#L1651-L1661

Do you remember why it was disabled in the first place @albertvillanova ?
Also cc @mariosasko @NielsRogge 


Comments: As discussed, I remember I set `decoded=False` here to avoid decoding just by iterating over examples of dataset. We wanted to decode only if the "audio" field (for Audio feature) was accessed.

> I set decoded=False here to avoid decoding just by iterating over examples of dataset. We wanted to decode only if the "audio" field (for Audio feature) was accessed

https://github.com/huggingface/datasets/pull/3430 will add more control to decoding, so I think it's OK to enable decoding in `__iter__` for now. After we merge the linked PR, the user can easily disable it again.

@mariosasko I wonder why there is no issue in `Audio` feature with decoding disabled in `__iter__`, whereas there is in `Image` feature.

Enabling decoding in `__iter__` will make fail Audio regressions tests: https://github.com/huggingface/datasets/runs/4608657230?check_suite_focus=true
```
=========================== short test summary info ============================
FAILED tests/features/test_audio.py::test_dataset_with_audio_feature_map_is_not_decoded
FAILED tests/features/test_audio.py::test_dataset_with_audio_feature_map_is_decoded
========================= 2 failed, 15 passed in 8.37s =========================

Please also note that the regression tests were implemented in accordance with the specifications:
- when doing a `map` (wich calls `__iter__`) of a function that doesn't access the audio field, the decoding should be disabled; this is why the decoding is disabled in `__iter__` (and only enabled in `__getitem__`).

>  I wonder why there is no issue in Audio feature with decoding disabled in __iter__, whereas there is in Image feature.

@albertvillanova Not sure if I understand this part. Currently, both the Image and the Audio feature don't decode data in `__iter__`, so their behavior is aligned there.


Therefore, this is not an issue, neither for Audio nor Image feature.

Could you please elaborate more on the expected use case? @lhoestq @NielsRogge 

The expected use cases (in accordance with the specs: see #2324):
- decoding should be enabled when accessing a specific item (`__getitem__`)
- decoding should be disabled while iterating (`__iter__`) to allow preprocessing of non-audio/image features (like label or text, for example) using `.map`
-  decoding should be enabled in a `.map` only if the `.map` function accesses the audio/image feature (implemented using `LazyDict`)

For me it's not an issue, actually. I just (mistakenly) tried to iterate over a PyTorch Dataset instead of a PyTorch DataLoader, 

i.e. I did this:

`batch = next(iter(train_ds)) `

whereas I actually wanted to do

`batch = next(iter(train_dataloader))`

and then it turned out that in the first case, the image was a string of bytes rather than a Pillow image, hence Quentin opened an issue.

Thanks @NielsRogge for the context.

So IMO everything is working as expected.

I'm closing this issue. Feel free to reopen it again if further changes of the specs should be addressed.

Thanks for the details :)

I still think that it's unexpected to get different results when doing
```python
for i in range(len(dataset)):
    sample = dataset[i]
```
and
```python
for sample in dataset:
    pass
```
even though I understand that if you don't need to decode the data, then decoding image or audio data when iterating is a waste of time and resources.

But in this case users can still drop the column that need decoding to get the full speed back no ?