Skip to content

Raise TypeError instead of panic on doing fetch_tile from striped TIFFs #99

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 13, 2025
Merged

Raise TypeError instead of panic on doing fetch_tile from striped TIFFs #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4106c3
Raise TypeError instead of panic when reading striped TIFFs
weiji14 May 13, 2025
3ba5eff
Add test to ensure TypeError raised when fetching tiles from striped TIF
weiji14 May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions python/src/tiff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use async_tiff::metadata::{PrefetchBuffer, TiffMetadataReader};
use async_tiff::reader::AsyncFileReader;
use async_tiff::TIFF;
use pyo3::exceptions::{PyFileNotFoundError, PyIndexError};
use pyo3::exceptions::{PyFileNotFoundError, PyIndexError, PyTypeError};
use pyo3::prelude::*;
use pyo3::types::PyType;
use pyo3_async_runtimes::tokio::future_into_py;
Expand Down Expand Up @@ -69,7 +69,11 @@ impl PyTIFF {
// TODO: avoid this clone; add Arc to underlying rust code?
.clone();
future_into_py(py, async move {
let tile = ifd.fetch_tile(x, y, reader.as_ref()).await.unwrap();
let tile = ifd
.fetch_tile(x, y, reader.as_ref())
.await
.map_err(|err| PyTypeError::new_err(err.to_string()))?;

Comment on lines +72 to +76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we'd have an error enum with known variants so that we can check specifically for "trying to load a strip tiff as a tiled tiff" and return a Python TypeError for that specific case.

But this is fine for now.

Ok(PyTile::new(tile))
})
}
Expand All @@ -91,7 +95,10 @@ impl PyTIFF {
// TODO: avoid this clone; add Arc to underlying rust code?
.clone();
future_into_py(py, async move {
let tiles = ifd.fetch_tiles(&x, &y, reader.as_ref()).await.unwrap();
let tiles = ifd
.fetch_tiles(&x, &y, reader.as_ref())
.await
.map_err(|err| PyTypeError::new_err(err.to_string()))?;
let py_tiles = tiles.into_iter().map(PyTile::new).collect::<Vec<_>>();
Ok(py_tiles)
})
Expand Down
22 changes: 22 additions & 0 deletions python/tests/test_exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"""
Unit tests to ensure that proper errors are raised instead of a panic.
"""

import pytest
from async_tiff.store import HTTPStore

from async_tiff import TIFF


async def test_raise_typeerror_fetch_tile_striped_tiff():
"""
Ensure that a TypeError is raised when trying to fetch a tile from a striped TIFF.
"""
store = HTTPStore(url="https://github.com/")
path = "OSGeo/gdal/raw/refs/tags/v3.11.0/autotest/gdrivers/data/gtiff/int8.tif"

tiff = await TIFF.open(path=path, store=store)
assert len(tiff.ifds) >= 1

with pytest.raises(TypeError):
await tiff.fetch_tile(0, 0, 0)