From 5517b7fc7c1c32340857e7b239ebca1f332d8ce8 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 13:21:37 +1200 Subject: [PATCH 01/13] chore: return Result<> for Cell --- README.md | 4 +- benches/encode_point.rs | 2 +- benches/get_cell_area.rs | 2 +- benches/get_resolution.rs | 2 +- benches/main.rs | 4 +- src/cells.rs | 123 ++++++++++-------- src/errors.rs | 1 + src/test/cells.rs | 256 +++++++++++++++++++++++--------------- src/tiles.rs | 3 +- 9 files changed, 235 insertions(+), 162 deletions(-) diff --git a/README.md b/README.md index 448e9be..9f9a7d4 100644 --- a/README.md +++ b/README.md @@ -37,10 +37,10 @@ let longitude = -3.7038; let latitude = 40.4168; let resolution = 10_u8; let qb = Cell::from_point(latitude, longitude, resolution); -assert_eq!(qb, Cell::new(5234261499580514303_u64)); +assert_eq!(qb, Cell::try_from(5234261499580514303_u64)); // Get a point from a Quadbin cell -let coords = Cell::new(5209574053332910079_u64).to_point(); +let coords = Cell::try_from(5209574053332910079_u64).to_point(); assert_eq!(coords, [-11.178401873711776, 33.75]); // Quadbin resolution at equator in mยฒ diff --git a/benches/encode_point.rs b/benches/encode_point.rs index 75f04b1..d4ac5fa 100644 --- a/benches/encode_point.rs +++ b/benches/encode_point.rs @@ -14,7 +14,7 @@ pub fn bench(c: &mut Criterion) { b.iter(|| black_box(&index)) }); group.bench_function("qbin", |b| { - let index = Cell::from_point(LAT, LNG, 12); + let index = Cell::from_point(LAT, LNG, 12).expect("cell index"); b.iter(|| black_box(&index)) }); diff --git a/benches/get_cell_area.rs b/benches/get_cell_area.rs index 4937f8f..41e1ae9 100644 --- a/benches/get_cell_area.rs +++ b/benches/get_cell_area.rs @@ -54,7 +54,7 @@ pub fn bench(c: &mut Criterion) { // Benchmark each resolution for quadbin for (i, &qb_index) in QUADBINS.iter().enumerate() { group.bench_with_input(BenchmarkId::new("qbin", i), &qb_index, |b, &index| { - let cell = Cell::new(index); + let cell = Cell::try_from(index).expect("cell index"); b.iter(|| black_box(cell).area_m2()) }); } diff --git a/benches/get_resolution.rs b/benches/get_resolution.rs index 48aea63..7585d22 100644 --- a/benches/get_resolution.rs +++ b/benches/get_resolution.rs @@ -13,7 +13,7 @@ pub fn bench(c: &mut Criterion) { b.iter(|| black_box(index).resolution()) }); group.bench_function("qbin", |b| { - let index = Cell::new(INPUT_QB); + let index = Cell::try_from(INPUT_QB).expect("cell index"); b.iter(|| black_box(index).resolution()) }); diff --git a/benches/main.rs b/benches/main.rs index 896e2e7..165f81d 100644 --- a/benches/main.rs +++ b/benches/main.rs @@ -1,13 +1,13 @@ use criterion::{criterion_group, criterion_main}; mod encode_point; -mod get_cell_area; +// mod get_cell_area; mod get_resolution; criterion_group!( benches, get_resolution::bench, - get_cell_area::bench, + // get_cell_area::bench, encode_point::bench ); diff --git a/src/cells.rs b/src/cells.rs index f9c8e8c..cb48c34 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -1,5 +1,8 @@ use crate::Direction; use crate::constants::*; +use crate::errors; +use crate::errors::InvalidCell; +use crate::errors::InvalidDirection; use crate::tiles::*; use crate::utils::*; use core::{fmt, num::NonZeroU64}; @@ -38,12 +41,8 @@ impl Cell { /// ``` /// let qb_cell = qbin::Cell::new(5234261499580514303); /// ``` - pub fn new(value: u64) -> Cell { - assert!( - is_valid_cell(value), - "Provided Quadbin Cell index is invalid" - ); - Cell(NonZeroU64::new(value).expect("non-zero cell index")) + pub fn new(value: u64) -> Result { + Cell::try_from(value) } /// Quadbin cell index validation. @@ -77,51 +76,51 @@ impl Cell { /// let parent = qb_cell.parent(2_u8); /// assert_eq!(parent, qbin::Cell::new(5200813144682790911)) /// ``` - pub fn parent(&self, parent_res: u8) -> Cell { + pub fn parent(&self, parent_res: u8) -> Result { cell_to_parent(self, parent_res) } // TODO: // Add child and/or children - /// Find the Cell's neighbor in a specific [Direction]. - /// - /// In the original JavaScript implementation, this operation is called - /// sibling. However, following the H3 naming convention, we decided - /// to name sibling's as neighbors. - /// - /// See [Direction] for allowed arguments. - /// - /// # Example - /// ``` - /// use qbin::{Cell, Direction}; - /// - /// let sibling = Cell::new(5209574053332910079).neighbor(Direction::Right); - /// assert_eq!(sibling, Some(Cell::new(5209626829891043327))); - /// ``` - pub fn neighbor(&self, direction: Direction) -> Option { - let tile = self.to_tile().neighbor(direction); - tile.map(Tile::to_cell) - } - - /// Find the Cell's sibling in a specific [Direction]. - /// - /// See [Cell::neighbor]. - pub fn sibling(&self, direction: Direction) -> Option { - let tile = self.to_tile().neighbor(direction); - tile.map(Tile::to_cell) - } - - /// List all Cell's neighbors. - pub fn neighbors(&self) -> [Option; 4] { - let mut neighbors = [None; 4]; - - for (i, neighbor) in neighbors.iter_mut().enumerate() { - *neighbor = self.neighbor(Direction::new_unchecked(i as u8)); - } - - neighbors - } + // /// Find the Cell's neighbor in a specific [Direction]. + // /// + // /// In the original JavaScript implementation, this operation is called + // /// sibling. However, following the H3 naming convention, we decided + // /// to name sibling's as neighbors. + // /// + // /// See [Direction] for allowed arguments. + // /// + // /// # Example + // /// ``` + // /// use qbin::{Cell, Direction}; + // /// + // /// let sibling = Cell::new(5209574053332910079).neighbor(Direction::Right); + // /// assert_eq!(sibling, Some(Cell::new(5209626829891043327))); + // /// ``` + // pub fn neighbor(&self, direction: Direction) -> Result { + // let tile = self.to_tile().neighbor(direction); + // tile.map(Tile::to_cell) + // } + + // /// Find the Cell's sibling in a specific [Direction]. + // /// + // /// See [Cell::neighbor]. + // pub fn sibling(&self, direction: Direction) -> Result { + // let tile = self.to_tile().neighbor(direction); + // tile.to_cell() + // } + + // /// List all Cell's neighbors. + // pub fn neighbors(&self) -> [Option; 4] { + // let mut neighbors = [None; 4]; + + // for (i, neighbor) in neighbors.iter_mut().enumerate() { + // *neighbor = self.neighbor(Direction::new_unchecked(i as u8)); + // } + + // neighbors + // } // TODO: // Add `direction_to_neighbor` -- return Direction to neighbor @@ -203,7 +202,7 @@ impl Cell { /// let cell = qbin::Cell::from_point(-41.28303675124842, 174.77727344223067, 26); /// assert_eq!(cell.get(), 5309133744805926483_u64) /// ``` - pub fn from_point(lat: f64, lng: f64, res: u8) -> Cell { + pub fn from_point(lat: f64, lng: f64, res: u8) -> Result { point_to_cell(lat, lng, res) } @@ -219,13 +218,27 @@ impl fmt::Display for Cell { } } +impl TryFrom for Cell { + type Error = errors::InvalidCell; + + fn try_from(value: u64) -> Result { + if !is_valid_cell(value) { + return Err(Self::Error::new( + Some(value), + "Provided Quadbin Cell index is invalid", + )); + } + + Ok(Self(NonZeroU64::new(value).expect("non-zero cell index"))) + } +} + // TODO: // Detect direction from neighbor https://github.com/HydroniumLabs/h3o/blob/ad2bebf52eab218d66b0bf213b14a2802bf616f7/src/base_cell.rs#L135C1-L150C6 // Internal functions ------------------------------------------------ - /// Quadbin cell validation -pub(crate) fn is_valid_cell(cell64: u64) -> bool { +fn is_valid_cell(cell64: u64) -> bool { let header = HEADER; let mode = (cell64 >> 59) & 7; let resolution = (cell64 >> 52) & 0x1F; @@ -241,7 +254,7 @@ pub(crate) fn is_valid_cell(cell64: u64) -> bool { } /// Convert a tile into a Quadbin cell. -pub(crate) fn tile_to_cell(tile: Tile) -> Cell { +pub(crate) fn tile_to_cell(tile: Tile) -> Result { let mut x = tile.x as u64; let mut y = tile.y as u64; let z = tile.z as u64; @@ -265,11 +278,11 @@ pub(crate) fn tile_to_cell(tile: Tile) -> Cell { y = (y | (y << S[0])) & B[0]; let cell = HEADER | (1 << 59) | (z << 52) | ((x | (y << 1)) >> 12) | (FOOTER >> (z * 2)); - Cell::new(cell) + Cell::try_from(cell) } /// Convert Quadbin cell into a tile -pub(crate) fn cell_to_tile(cell: &Cell) -> Tile { +fn cell_to_tile(cell: &Cell) -> Tile { assert!(cell.is_valid(), "Quadbin cell index is not valid"); let cell64 = cell.get(); @@ -303,7 +316,7 @@ pub(crate) fn cell_to_tile(cell: &Cell) -> Tile { } /// Convert a geographic point into a cell. -pub(crate) fn point_to_cell(lat: f64, lng: f64, res: u8) -> Cell { +fn point_to_cell(lat: f64, lng: f64, res: u8) -> Result { let lng = clip_longitude(lng); let lat = clip_latitude(lat); @@ -313,7 +326,7 @@ pub(crate) fn point_to_cell(lat: f64, lng: f64, res: u8) -> Cell { } /// Convert cell into point -pub(crate) fn cell_to_point(cell: &Cell) -> [f64; 2] { +fn cell_to_point(cell: &Cell) -> [f64; 2] { assert!(cell.is_valid(), "Quadbin cell index is not valid"); let tile = cell.to_tile(); @@ -326,7 +339,7 @@ pub(crate) fn cell_to_point(cell: &Cell) -> [f64; 2] { } /// Compute the parent cell for a specific resolution. -pub(crate) fn cell_to_parent(cell: &Cell, parent_res: u8) -> Cell { +fn cell_to_parent(cell: &Cell, parent_res: u8) -> Result { // Check resolution let resolution = cell.resolution(); assert!( @@ -338,5 +351,5 @@ pub(crate) fn cell_to_parent(cell: &Cell, parent_res: u8) -> Cell { | ((parent_res as u64) << 52) | (FOOTER >> ((parent_res as u64) << 1)); - Cell::new(result) + Cell::try_from(result) } diff --git a/src/errors.rs b/src/errors.rs index 5e72eca..1a1e606 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -45,3 +45,4 @@ macro_rules! invalid_value_error { } invalid_value_error!("direction", InvalidDirection, u8); +invalid_value_error!("cell index", InvalidCell, Option); diff --git a/src/test/cells.rs b/src/test/cells.rs index 9b09b14..66f5315 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -1,5 +1,6 @@ use crate::cells::*; use crate::directions::Direction; +use crate::errors::*; use crate::tiles::*; use approx::assert_relative_eq; @@ -9,29 +10,6 @@ const DOWN: Direction = Direction::Down; const RIGHT: Direction = Direction::Right; const LEFT: Direction = Direction::Left; -// Test validiy of Quadbin cell indexes -#[test] -fn test_is_cell_valid() { - let cases = [ - (5209574053332910079_u64, true), - (5192650370358181887_u64, true), - (5202361257054699519_u64, true), - (5291729562728627583_u64, true), - ]; - - for (cell, expected) in cases.iter() { - assert_eq!(Cell::new(*cell).is_valid(), *expected); - } -} - -// Expect panic due to invalid cell index -#[test] -#[should_panic(expected = "Provided Quadbin Cell index is invalid")] -fn test_new_invalid_cell() { - let _ = Cell::new(5209574053332910078_u64); - let _ = Cell::new(6362495557939757055_u64); -} - // Validation test from original CARTO's `quadbin-js` // https://github.com/CartoDB/quadbin-js/blob/40cce2fc6b9dc72bf19c69ffb6705f8b73d24b2c/test/index.spec.ts#L30-L34 #[test] @@ -46,12 +24,18 @@ fn test_tile_and_cell_conversion() { // Tile to cell conversion for (x, y, z, cell) in cases.iter() { - assert_eq!(Tile::new(*x, *y, *z).to_cell(), Cell::new(*cell)); + assert_eq!( + Tile::new(*x, *y, *z).to_cell().expect("cell index"), + Cell::try_from(*cell).expect("cell index") + ); } // Cell to tile conversion for (x, y, z, cell) in cases.iter() { - assert_eq!(Cell::new(*cell).to_tile(), Tile::new(*x, *y, *z)); + assert_eq!( + Cell::try_from(*cell).expect("cell index").to_tile(), + Tile::new(*x, *y, *z) + ); } } @@ -76,7 +60,10 @@ fn test_point_to_cell() { ]; for (x, y, res, cell) in cases.iter() { - assert_eq!(Cell::from_point(*y, *x, *res), Cell::new(*cell)); + assert_eq!( + Cell::from_point(*y, *x, *res).expect("cell index"), + Cell::try_from(*cell).expect("cell index") + ); } } @@ -85,7 +72,9 @@ fn test_point_to_cell() { fn test_cell_to_bbox() { // Conversion works assert_eq!( - Cell::new(5209574053332910079).to_bbox(), + Cell::try_from(5209574053332910079) + .expect("cell index") + .to_bbox(), [22.5, -21.943045533438166, 45.0, 0.0] ); @@ -98,7 +87,7 @@ fn test_cell_to_bbox() { ]; for i in cases.iter() { - let bbox = Cell::new(*i).to_bbox(); + let bbox = Cell::try_from(*i).expect("cell index").to_bbox(); assert!(bbox[0] < bbox[2]); assert!(bbox[1] < bbox[3]); } @@ -108,11 +97,15 @@ fn test_cell_to_bbox() { #[test] fn test_cell_to_point() { assert_eq!( - Cell::new(5209574053332910079_u64).to_point(), + Cell::try_from(5209574053332910079_u64) + .expect("cell index") + .to_point(), [-11.178401873711776, 33.75] ); - let coords = Cell::new(5309133744805926483_u64).to_point(); + let coords = Cell::try_from(5309133744805926483_u64) + .expect("cell index") + .to_point(); assert_relative_eq!(coords[0], -41.28303708488909, epsilon = 1e-6); assert_relative_eq!(coords[1], 174.77727502584457, epsilon = 1e-6) } @@ -120,7 +113,7 @@ fn test_cell_to_point() { // Get cell resolution #[test] fn test_get_cell_resolution() { - let qb_cell = Cell::new(5209574053332910079_u64); + let qb_cell = Cell::try_from(5209574053332910079_u64).expect("cell index"); assert_eq!(qb_cell.resolution(), 4_u8) } @@ -133,93 +126,158 @@ fn test_cell_to_parent() { ]; for (cell, res, parent) in cases.iter() { - assert_eq!(Cell::new(*cell).parent(*res), Cell::new(*parent)); + assert_eq!( + Cell::try_from(*cell).expect("cell index").parent(*res), + Cell::try_from(*parent) + ); } } #[test] #[should_panic(expected = "parent resolution should be greater than current resolution")] fn test_cell_to_parent_invalid_resolution() { - let cell = Cell::new(5209574053332910079); + let cell = Cell::try_from(5209574053332910079).expect("cell index"); let _ = cell.parent(4); } // Estimate cell area #[test] fn test_cell_area() { - let area = Cell::new(5209574053332910079_u64).area_m2(); + let area = Cell::try_from(5209574053332910079_u64) + .expect("cell index") + .area_m2(); assert_relative_eq!(area, 6023040823252.664, epsilon = 1e-2); } -// Find cell's neighbors -// Identical to -// https://github.com/CartoDB/quadbin-py/blob/39a0adbb238ff214fbbca7b73200cfebf2aef38c/tests/unit/test_main.py#L203 -#[test] -fn test_cell_neighbor() { - assert_eq!(Cell::new(5192650370358181887).neighbor(UP), None); - assert_eq!(Cell::new(5193776270265024511).neighbor(UP), None); - assert_eq!(Cell::new(5194902170171867135).neighbor(UP), None); - assert_eq!(Cell::new(5194902170171867135).neighbor(RIGHT), None); +// // Find cell's neighbors +// // Identical to +// // https://github.com/CartoDB/quadbin-py/blob/39a0adbb238ff214fbbca7b73200cfebf2aef38c/tests/unit/test_main.py#L203 +// #[test] +// fn test_cell_neighbor() { +// assert_eq!( +// Cell::try_from(5192650370358181887) +// .expect("cell index") +// .neighbor(UP), +// None +// ); +// assert_eq!( +// Cell::try_from(5193776270265024511) +// .expect("cell index") +// .neighbor(UP), +// None +// ); +// assert_eq!( +// Cell::try_from(5194902170171867135) +// .expect("cell index") +// .neighbor(UP), +// None +// ); +// assert_eq!( +// Cell::try_from(5194902170171867135) +// .expect("cell index") +// .neighbor(RIGHT), +// None +// ); - // Resolution 1 - assert_eq!( - Cell::new(5193776270265024511).neighbor(DOWN), - Some(Cell::new(5196028070078709759)) - ); - assert_eq!( - Cell::new(5193776270265024511).neighbor(RIGHT), - Some(Cell::new(5194902170171867135)) - ); - assert_eq!( - Cell::new(5194902170171867135).neighbor(DOWN), - Some(Cell::new(5197153969985552383)) - ); - assert_eq!( - Cell::new(5194902170171867135).neighbor(LEFT), - Some(Cell::new(5193776270265024511)) - ); - assert_eq!( - Cell::new(5209574053332910079).neighbor(UP), - Some(Cell::new(5208061125333090303)) - ); +// // Resolution 1 +// assert_eq!( +// Cell::try_from(5193776270265024511) +// .expect("cell index") +// .neighbor(DOWN), +// Some(Cell::try_from(5196028070078709759).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5193776270265024511) +// .expect("cell index") +// .neighbor(RIGHT), +// Some(Cell::try_from(5194902170171867135).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5194902170171867135) +// .expect("cell index") +// .neighbor(DOWN), +// Some(Cell::try_from(5197153969985552383).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5194902170171867135) +// .expect("cell index") +// .neighbor(LEFT), +// Some(Cell::try_from(5193776270265024511).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5209574053332910079) +// .expect("cell index") +// .neighbor(UP), +// Some(Cell::try_from(5208061125333090303).expect("cell index")) +// ); - // Resolution 4 - assert_eq!( - Cell::new(5209574053332910079).neighbor(DOWN), - Some(Cell::new(5209609237704998911)) +// // Resolution 4 +// assert_eq!( +// Cell::try_from(5209574053332910079) +// .expect("cell index") +// .neighbor(DOWN), +// Some(Cell::try_from(5209609237704998911).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5209574053332910079) +// .expect("cell index") +// .neighbor(LEFT), +// Some(Cell::try_from(5209556461146865663).expect("cell index")) +// ); +// assert_eq!( +// Cell::try_from(5209574053332910079) +// .expect("cell index") +// .neighbor(RIGHT), +// Some(Cell::try_from(5209626829891043327).expect("cell index")) +// ); +// } + +// // List all Cell's neighbors +// #[test] +// fn test_cell_neighbors() { +// let center_cells = [ +// Cell::try_from(5209574053332910079).expect("cell index"), +// Cell::try_from(5194902170171867135).expect("cell index"), +// Cell::try_from(5192650370358181887).expect("cell index"), +// Cell::try_from(5201094619659501567).expect("cell index"), +// ]; + +// for i in center_cells.iter() { +// assert_eq!( +// i.neighbors(), +// [ +// i.neighbor(UP), +// i.neighbor(RIGHT), +// i.neighbor(LEFT), +// i.neighbor(DOWN) +// ] +// ) +// } + +// // Test that None is returned alongside with Some(Cell) +// let nn = Cell::try_from(5201094619659501567) +// .expect("cell index") +// .neighbors(); +// assert_eq!(nn[1], None) +// } + +#[test] +fn test_invalid_cellindex() { + assert!( + !InvalidCell::new(Some(5209574053332910078_u64), "error") + .to_string() + .is_empty() ); assert_eq!( - Cell::new(5209574053332910079).neighbor(LEFT), - Some(Cell::new(5209556461146865663)) + Cell::try_from(5209574053332910078_u64).err(), + Some(InvalidCell::new( + Some(5209574053332910078_u64), + "Provided Quadbin Cell index is invalid" + )) ); assert_eq!( - Cell::new(5209574053332910079).neighbor(RIGHT), - Some(Cell::new(5209626829891043327)) + Cell::try_from(5209574053332910079_u64) + .expect("cell index") + .get(), + 5209574053332910079_u64 ); } - -// List all Cell's neighbors -#[test] -fn test_cell_neighbors() { - let center_cells = [ - Cell::new(5209574053332910079), - Cell::new(5194902170171867135), - Cell::new(5192650370358181887), - Cell::new(5201094619659501567), - ]; - - for i in center_cells.iter() { - assert_eq!( - i.neighbors(), - [ - i.neighbor(UP), - i.neighbor(RIGHT), - i.neighbor(LEFT), - i.neighbor(DOWN) - ] - ) - } - - // Test that None is returned alongside with Some(Cell) - let nn = Cell::new(5201094619659501567).neighbors(); - assert_eq!(nn[1], None) -} diff --git a/src/tiles.rs b/src/tiles.rs index 2974109..a5de61a 100644 --- a/src/tiles.rs +++ b/src/tiles.rs @@ -1,5 +1,6 @@ use crate::Direction; use crate::cells::*; +use crate::errors::InvalidCell; use crate::utils::*; /// A single tile coordinates @@ -19,7 +20,7 @@ impl Tile { } /// Convert to Quadbin cell. - pub fn to_cell(self) -> Cell { + pub fn to_cell(self) -> Result { tile_to_cell(self) } From 3750fe661c46ded040bcabaf370d1d38849d5cd4 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:08:20 +1200 Subject: [PATCH 02/13] chore: idiomatic Result<> or Option<> as a return values --- README.md | 8 +- src/cells.rs | 146 +++++++++++++++---------------- src/test/cells.rs | 212 ++++++++++++++++++++++------------------------ 3 files changed, 180 insertions(+), 186 deletions(-) diff --git a/README.md b/README.md index 9f9a7d4..d5b4654 100644 --- a/README.md +++ b/README.md @@ -36,15 +36,15 @@ use approx::assert_relative_eq; let longitude = -3.7038; let latitude = 40.4168; let resolution = 10_u8; -let qb = Cell::from_point(latitude, longitude, resolution); -assert_eq!(qb, Cell::try_from(5234261499580514303_u64)); +let qb = Cell::from_point(latitude, longitude, resolution).expect("cell index"); +assert_eq!(qb, Cell::try_from(5234261499580514303_u64).expect("cell index")); // Get a point from a Quadbin cell -let coords = Cell::try_from(5209574053332910079_u64).to_point(); +let coords = Cell::try_from(5209574053332910079_u64).expect("cell index").to_point(); assert_eq!(coords, [-11.178401873711776, 33.75]); // Quadbin resolution at equator in mยฒ -let area = Cell::from_point(0.0, 0.0, 26).area_m2(); +let area = Cell::from_point(0.0, 0.0, 26).expect("cell index").area_m2(); assert_relative_eq!(area, 0.36, epsilon = 1e-2) ``` diff --git a/src/cells.rs b/src/cells.rs index cb48c34..e10fe19 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -2,7 +2,6 @@ use crate::Direction; use crate::constants::*; use crate::errors; use crate::errors::InvalidCell; -use crate::errors::InvalidDirection; use crate::tiles::*; use crate::utils::*; use core::{fmt, num::NonZeroU64}; @@ -37,30 +36,21 @@ impl Cell { /// Create new Quadbin cell from index. /// - /// # Example - /// ``` - /// let qb_cell = qbin::Cell::new(5234261499580514303); - /// ``` - pub fn new(value: u64) -> Result { - Cell::try_from(value) - } - - /// Quadbin cell index validation. + /// A shortcut for [Cell::try_from()]. /// /// # Example /// ``` /// let qb_cell = qbin::Cell::new(5234261499580514303); - /// assert_eq!(qb_cell.is_valid(), true) /// ``` - pub fn is_valid(&self) -> bool { - is_valid_cell(self.get()) + pub fn new(value: u64) -> Self { + Cell::try_from(value).expect("cell index") } /// Returns the resolution of the cell index. /// /// # Example /// ``` - /// let qb_cell = qbin::Cell::new(5234261499580514303); + /// let qb_cell = qbin::Cell::try_from(5234261499580514303).expect("cell index"); /// let res = qb_cell.resolution(); /// assert_eq!(res, 10) /// ``` @@ -72,55 +62,61 @@ impl Cell { /// /// # Example /// ``` - /// let qb_cell = qbin::Cell::new(5209574053332910079); - /// let parent = qb_cell.parent(2_u8); - /// assert_eq!(parent, qbin::Cell::new(5200813144682790911)) + /// use qbin::Cell; + /// + /// let qb_cell = Cell::try_from(5209574053332910079).expect("cell index"); + /// let parent = qb_cell.parent(2_u8).expect("cell index"); + /// assert_eq!(parent, qbin::Cell::try_from(5200813144682790911).expect("cell index")) /// ``` - pub fn parent(&self, parent_res: u8) -> Result { + // TODO: + // Return Option as in neighbor + pub fn parent(&self, parent_res: u8) -> Result { cell_to_parent(self, parent_res) } // TODO: // Add child and/or children - // /// Find the Cell's neighbor in a specific [Direction]. - // /// - // /// In the original JavaScript implementation, this operation is called - // /// sibling. However, following the H3 naming convention, we decided - // /// to name sibling's as neighbors. - // /// - // /// See [Direction] for allowed arguments. - // /// - // /// # Example - // /// ``` - // /// use qbin::{Cell, Direction}; - // /// - // /// let sibling = Cell::new(5209574053332910079).neighbor(Direction::Right); - // /// assert_eq!(sibling, Some(Cell::new(5209626829891043327))); - // /// ``` - // pub fn neighbor(&self, direction: Direction) -> Result { - // let tile = self.to_tile().neighbor(direction); - // tile.map(Tile::to_cell) - // } - - // /// Find the Cell's sibling in a specific [Direction]. - // /// - // /// See [Cell::neighbor]. - // pub fn sibling(&self, direction: Direction) -> Result { - // let tile = self.to_tile().neighbor(direction); - // tile.to_cell() - // } - - // /// List all Cell's neighbors. - // pub fn neighbors(&self) -> [Option; 4] { - // let mut neighbors = [None; 4]; - - // for (i, neighbor) in neighbors.iter_mut().enumerate() { - // *neighbor = self.neighbor(Direction::new_unchecked(i as u8)); - // } - - // neighbors - // } + /// Find the Cell's neighbor in a specific [Direction]. + /// + /// In the original JavaScript implementation, this operation is called + /// sibling. However, following the H3 naming convention, we decided + /// to name sibling's as neighbors. + /// + /// See [Direction] for allowed arguments. + /// + /// Return `None` if there is no neighbor in this [Direction]. + /// + /// # Example + /// ``` + /// use qbin::{Cell, Direction}; + /// + /// let cell = Cell::try_from(5209574053332910079).expect("cell index"); + /// let sibling = cell.neighbor(Direction::Right); + /// assert_eq!(sibling, Some(Cell::new(5209626829891043327))); + /// ``` + pub fn neighbor(&self, direction: Direction) -> Option { + let tile = self.to_tile().neighbor(direction)?; + Some(tile.to_cell().expect("cell index")) + } + + /// Find the Cell's sibling in a specific [Direction]. + /// + /// See [Cell::neighbor]. + pub fn sibling(&self, direction: Direction) -> Option { + self.neighbor(direction) + } + + /// List all Cell's neighbors. + pub fn neighbors(&self) -> [Option; 4] { + let mut neighbors = [None; 4]; + + for (i, neighbor) in neighbors.iter_mut().enumerate() { + *neighbor = self.neighbor(Direction::new_unchecked(i as u8)); + } + + neighbors + } // TODO: // Add `direction_to_neighbor` -- return Direction to neighbor @@ -132,8 +128,10 @@ impl Cell { /// # Example /// ``` /// use approx::assert_relative_eq; + /// use qbin::Cell; /// - /// let area = qbin::Cell::new(5234261499580514303_u64).area_m2(); + /// let my_cell = Cell::try_from(5234261499580514303_u64).expect("cell index"); + /// let area = my_cell.area_m2(); /// assert_relative_eq!(area, 888546364.7859862, epsilon = 1e-6) /// /// ``` @@ -148,8 +146,10 @@ impl Cell { /// # Example /// ``` /// use approx::assert_relative_eq; + /// use qbin::Cell; /// - /// let area = qbin::Cell::new(5234261499580514303_u64).area_km2(); + /// let my_cell = Cell::try_from(5234261499580514303_u64).expect("cell index"); + /// let area = my_cell.area_km2(); /// assert_relative_eq!(area, 888.5463647859862, epsilon = 1e-6) /// /// ``` @@ -165,7 +165,8 @@ impl Cell { /// ``` /// use qbin::Cell; /// - /// let coords = Cell::new(5209574053332910079_u64).to_point(); + /// let cell = Cell::try_from(5209574053332910079).expect("cell index"); + /// let coords = cell.to_point(); /// assert_eq!(coords, [-11.178401873711776, 33.75]); /// ``` /// @@ -180,18 +181,21 @@ impl Cell { /// /// # Example /// ``` - /// let bbox = qbin::Cell::new(5209574053332910079).to_bbox(); + /// use qbin::Cell; + /// + /// let cell = Cell::try_from(5209574053332910079).expect("cell index"); + /// let bbox = cell.to_bbox(); /// assert_eq!( bbox, [22.5, -21.943045533438166, 45.0, 0.0]) /// ``` pub fn to_bbox(&self) -> [f64; 4] { - let tile = self.to_tile(); + let tile = &self.to_tile(); - let xmin = tile.to_longitude(0.0); - let xmax = tile.to_longitude(1.0); - let ymin = tile.to_latitude(1.0); - let ymax = tile.to_latitude(0.0); + let xmin = &tile.to_longitude(0.0); + let xmax = &tile.to_longitude(1.0); + let ymin = &tile.to_latitude(1.0); + let ymax = &tile.to_latitude(0.0); - [xmin, ymin, xmax, ymax] + [*xmin, *ymin, *xmax, *ymax] } /// Convert a geographic point into a Quadbin cell. @@ -199,10 +203,12 @@ impl Cell { /// # Example /// /// ``` - /// let cell = qbin::Cell::from_point(-41.28303675124842, 174.77727344223067, 26); + /// use qbin::Cell; + /// + /// let cell = Cell::from_point(-41.28303675124842, 174.77727344223067, 26).expect("cell index"); /// assert_eq!(cell.get(), 5309133744805926483_u64) /// ``` - pub fn from_point(lat: f64, lng: f64, res: u8) -> Result { + pub fn from_point(lat: f64, lng: f64, res: u8) -> Result { point_to_cell(lat, lng, res) } @@ -283,8 +289,6 @@ pub(crate) fn tile_to_cell(tile: Tile) -> Result { /// Convert Quadbin cell into a tile fn cell_to_tile(cell: &Cell) -> Tile { - assert!(cell.is_valid(), "Quadbin cell index is not valid"); - let cell64 = cell.get(); let z = (cell64 >> 52) & 31; let q = (cell64 & FOOTER) << 12; @@ -327,8 +331,6 @@ fn point_to_cell(lat: f64, lng: f64, res: u8) -> Result { /// Convert cell into point fn cell_to_point(cell: &Cell) -> [f64; 2] { - assert!(cell.is_valid(), "Quadbin cell index is not valid"); - let tile = cell.to_tile(); let lat = tile.to_latitude(0.5); let lon = tile.to_longitude(0.5); diff --git a/src/test/cells.rs b/src/test/cells.rs index 66f5315..da8750c 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -72,9 +72,7 @@ fn test_point_to_cell() { fn test_cell_to_bbox() { // Conversion works assert_eq!( - Cell::try_from(5209574053332910079) - .expect("cell index") - .to_bbox(), + Cell::try_from(5209574053332910079).unwrap().to_bbox(), [22.5, -21.943045533438166, 45.0, 0.0] ); @@ -87,7 +85,7 @@ fn test_cell_to_bbox() { ]; for i in cases.iter() { - let bbox = Cell::try_from(*i).expect("cell index").to_bbox(); + let bbox = Cell::try_from(*i).unwrap().to_bbox(); assert!(bbox[0] < bbox[2]); assert!(bbox[1] < bbox[3]); } @@ -148,117 +146,111 @@ fn test_cell_area() { assert_relative_eq!(area, 6023040823252.664, epsilon = 1e-2); } -// // Find cell's neighbors -// // Identical to -// // https://github.com/CartoDB/quadbin-py/blob/39a0adbb238ff214fbbca7b73200cfebf2aef38c/tests/unit/test_main.py#L203 -// #[test] -// fn test_cell_neighbor() { -// assert_eq!( -// Cell::try_from(5192650370358181887) -// .expect("cell index") -// .neighbor(UP), -// None -// ); -// assert_eq!( -// Cell::try_from(5193776270265024511) -// .expect("cell index") -// .neighbor(UP), -// None -// ); -// assert_eq!( -// Cell::try_from(5194902170171867135) -// .expect("cell index") -// .neighbor(UP), -// None -// ); -// assert_eq!( -// Cell::try_from(5194902170171867135) -// .expect("cell index") -// .neighbor(RIGHT), -// None -// ); +// Find cell's neighbors +// Identical to +// https://github.com/CartoDB/quadbin-py/blob/39a0adbb238ff214fbbca7b73200cfebf2aef38c/tests/unit/test_main.py#L203 +#[test] +fn test_cell_neighbor() { + assert_eq!( + Cell::try_from(5192650370358181887) + .expect("cell index") + .neighbor(UP), + None + ); + assert_eq!( + Cell::try_from(5193776270265024511) + .expect("cell index") + .neighbor(UP), + None + ); + assert_eq!( + Cell::try_from(5194902170171867135) + .expect("cell index") + .neighbor(UP), + None + ); + assert_eq!( + Cell::try_from(5194902170171867135) + .expect("cell index") + .neighbor(RIGHT), + None + ); -// // Resolution 1 -// assert_eq!( -// Cell::try_from(5193776270265024511) -// .expect("cell index") -// .neighbor(DOWN), -// Some(Cell::try_from(5196028070078709759).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5193776270265024511) -// .expect("cell index") -// .neighbor(RIGHT), -// Some(Cell::try_from(5194902170171867135).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5194902170171867135) -// .expect("cell index") -// .neighbor(DOWN), -// Some(Cell::try_from(5197153969985552383).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5194902170171867135) -// .expect("cell index") -// .neighbor(LEFT), -// Some(Cell::try_from(5193776270265024511).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5209574053332910079) -// .expect("cell index") -// .neighbor(UP), -// Some(Cell::try_from(5208061125333090303).expect("cell index")) -// ); + // Resolution 1 + assert_eq!( + Cell::try_from(5193776270265024511) + .expect("cell index") + .neighbor(DOWN), + Some(Cell::try_from(5196028070078709759).expect("cell index")) + ); + assert_eq!( + Cell::try_from(5193776270265024511) + .expect("cell index") + .neighbor(RIGHT), + Some(Cell::new(5194902170171867135)) + ); + assert_eq!( + Cell::try_from(5194902170171867135) + .expect("cell index") + .neighbor(DOWN), + Some(Cell::new(5197153969985552383)) + ); + assert_eq!( + Cell::try_from(5194902170171867135) + .expect("cell index") + .neighbor(LEFT), + Some(Cell::new(5193776270265024511)) + ); + assert_eq!( + Cell::try_from(5209574053332910079) + .expect("cell index") + .neighbor(UP), + Some(Cell::new(5208061125333090303)) + ); -// // Resolution 4 -// assert_eq!( -// Cell::try_from(5209574053332910079) -// .expect("cell index") -// .neighbor(DOWN), -// Some(Cell::try_from(5209609237704998911).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5209574053332910079) -// .expect("cell index") -// .neighbor(LEFT), -// Some(Cell::try_from(5209556461146865663).expect("cell index")) -// ); -// assert_eq!( -// Cell::try_from(5209574053332910079) -// .expect("cell index") -// .neighbor(RIGHT), -// Some(Cell::try_from(5209626829891043327).expect("cell index")) -// ); -// } + // Resolution 4 + assert_eq!( + Cell::new(5209574053332910079).neighbor(DOWN), + Some(Cell::new(5209609237704998911)) + ); + assert_eq!( + Cell::new(5209574053332910079).neighbor(LEFT), + Some(Cell::new(5209556461146865663)) + ); + assert_eq!( + Cell::new(5209574053332910079).neighbor(RIGHT), + Some(Cell::new(5209626829891043327)) + ); +} -// // List all Cell's neighbors -// #[test] -// fn test_cell_neighbors() { -// let center_cells = [ -// Cell::try_from(5209574053332910079).expect("cell index"), -// Cell::try_from(5194902170171867135).expect("cell index"), -// Cell::try_from(5192650370358181887).expect("cell index"), -// Cell::try_from(5201094619659501567).expect("cell index"), -// ]; +// List all Cell's neighbors +#[test] +fn test_cell_neighbors() { + let center_cells = [ + Cell::try_from(5209574053332910079).expect("cell index"), + Cell::try_from(5194902170171867135).expect("cell index"), + Cell::try_from(5192650370358181887).expect("cell index"), + Cell::try_from(5201094619659501567).expect("cell index"), + ]; -// for i in center_cells.iter() { -// assert_eq!( -// i.neighbors(), -// [ -// i.neighbor(UP), -// i.neighbor(RIGHT), -// i.neighbor(LEFT), -// i.neighbor(DOWN) -// ] -// ) -// } + for i in center_cells.iter() { + assert_eq!( + i.neighbors(), + [ + i.neighbor(UP), + i.neighbor(RIGHT), + i.neighbor(LEFT), + i.neighbor(DOWN) + ] + ) + } -// // Test that None is returned alongside with Some(Cell) -// let nn = Cell::try_from(5201094619659501567) -// .expect("cell index") -// .neighbors(); -// assert_eq!(nn[1], None) -// } + // Test that None is returned alongside with Some(Cell) + let nn = Cell::try_from(5201094619659501567) + .expect("cell index") + .neighbors(); + assert_eq!(nn[1], None) +} #[test] fn test_invalid_cellindex() { From 9ce4feb1c830d63a8737e5908fe66be7ba24a7e3 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:15:18 +1200 Subject: [PATCH 03/13] refactor: minor relocation of code --- src/cells.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/cells.rs b/src/cells.rs index e10fe19..9f06253 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -28,6 +28,21 @@ use core::{fmt, num::NonZeroU64}; #[derive(Debug, PartialEq, Eq, Copy, Clone)] pub struct Cell(NonZeroU64); +impl TryFrom for Cell { + type Error = errors::InvalidCell; + + fn try_from(value: u64) -> Result { + if !is_valid_cell(value) { + return Err(Self::Error::new( + Some(value), + "Provided Quadbin Cell index is invalid", + )); + } + + Ok(Self(NonZeroU64::new(value).expect("non-zero cell index"))) + } +} + impl Cell { /// Returns the inner u64 value of the cell. pub fn get(&self) -> u64 { @@ -224,21 +239,6 @@ impl fmt::Display for Cell { } } -impl TryFrom for Cell { - type Error = errors::InvalidCell; - - fn try_from(value: u64) -> Result { - if !is_valid_cell(value) { - return Err(Self::Error::new( - Some(value), - "Provided Quadbin Cell index is invalid", - )); - } - - Ok(Self(NonZeroU64::new(value).expect("non-zero cell index"))) - } -} - // TODO: // Detect direction from neighbor https://github.com/HydroniumLabs/h3o/blob/ad2bebf52eab218d66b0bf213b14a2802bf616f7/src/base_cell.rs#L135C1-L150C6 @@ -346,7 +346,7 @@ fn cell_to_parent(cell: &Cell, parent_res: u8) -> Result { let resolution = cell.resolution(); assert!( parent_res < resolution, - "parent resolution should be greater than current resolution" + "parent resolution should be lower than current resolution" ); let result = (cell.get() & !(0x1F << 52)) From a2bccdf3cc855bf53c55b155cb18791f83f29496 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:23:44 +1200 Subject: [PATCH 04/13] typo: fix typo --- src/test/cells.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/cells.rs b/src/test/cells.rs index da8750c..54f113d 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -131,7 +131,7 @@ fn test_cell_to_parent() { } } #[test] -#[should_panic(expected = "parent resolution should be greater than current resolution")] +#[should_panic(expected = "parent resolution should be lower than current resolution")] fn test_cell_to_parent_invalid_resolution() { let cell = Cell::try_from(5209574053332910079).expect("cell index"); let _ = cell.parent(4); From 3d1e86e0560a7679ac6e1c0cc569d16d9b432f72 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:53:46 +1200 Subject: [PATCH 05/13] doc: small improvements in documentation --- src/cells.rs | 12 +++++++++--- src/lib.rs | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cells.rs b/src/cells.rs index 9f06253..90c612b 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -55,7 +55,11 @@ impl Cell { /// /// # Example /// ``` - /// let qb_cell = qbin::Cell::new(5234261499580514303); + /// use qbin::Cell; + /// + /// let cell_new = Cell::new(5234261499580514303); + /// let cell_try = Cell::try_from(5234261499580514303).expect("cell index"); + /// assert_eq!(cell_new, cell_try); /// ``` pub fn new(value: u64) -> Self { Cell::try_from(value).expect("cell index") @@ -65,7 +69,9 @@ impl Cell { /// /// # Example /// ``` - /// let qb_cell = qbin::Cell::try_from(5234261499580514303).expect("cell index"); + /// use qbin::Cell; + /// + /// let qb_cell = Cell::try_from(5234261499580514303).expect("cell index"); /// let res = qb_cell.resolution(); /// assert_eq!(res, 10) /// ``` @@ -81,7 +87,7 @@ impl Cell { /// /// let qb_cell = Cell::try_from(5209574053332910079).expect("cell index"); /// let parent = qb_cell.parent(2_u8).expect("cell index"); - /// assert_eq!(parent, qbin::Cell::try_from(5200813144682790911).expect("cell index")) + /// assert_eq!(parent, Cell::try_from(5200813144682790911).expect("cell index")) /// ``` // TODO: // Return Option as in neighbor diff --git a/src/lib.rs b/src/lib.rs index 0af4a97..a367a87 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,7 @@ #![doc = include_str!("../README.md")] // Quadbin cell itself -pub mod cells; +mod cells; pub use crate::cells::Cell; // Direction struct From e5fb4c61fa3dfab4e0817fcff4391f032234c989 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:54:06 +1200 Subject: [PATCH 06/13] bench: updated benchmarks --- benches/decode_point.rs | 21 +++++++++++++++++++++ benches/get_resolution.rs | 2 +- benches/main.rs | 4 +++- cliff.toml | 16 ++++++++-------- 4 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 benches/decode_point.rs diff --git a/benches/decode_point.rs b/benches/decode_point.rs new file mode 100644 index 0000000..093c934 --- /dev/null +++ b/benches/decode_point.rs @@ -0,0 +1,21 @@ +use criterion::{Criterion, black_box}; +use qbin::Cell; + +const QBIN: u64 = 5246083350086549503; +const GEOHASH: &str = "rbsm1hsuvshv"; + +pub fn bench(c: &mut Criterion) { + let mut group = c.benchmark_group("decodePoint"); + + group.bench_function("geohash", |b| { + let (index, _, _) = geohash::decode(GEOHASH).unwrap(); + b.iter(|| black_box(&index)) + }); + + group.bench_function("qbin", |b| { + let index = Cell::new(QBIN).to_point(); + b.iter(|| black_box(&index)) + }); + + group.finish(); +} diff --git a/benches/get_resolution.rs b/benches/get_resolution.rs index 7585d22..48aea63 100644 --- a/benches/get_resolution.rs +++ b/benches/get_resolution.rs @@ -13,7 +13,7 @@ pub fn bench(c: &mut Criterion) { b.iter(|| black_box(index).resolution()) }); group.bench_function("qbin", |b| { - let index = Cell::try_from(INPUT_QB).expect("cell index"); + let index = Cell::new(INPUT_QB); b.iter(|| black_box(index).resolution()) }); diff --git a/benches/main.rs b/benches/main.rs index 165f81d..ca1ef45 100644 --- a/benches/main.rs +++ b/benches/main.rs @@ -1,5 +1,6 @@ use criterion::{criterion_group, criterion_main}; +mod decode_point; mod encode_point; // mod get_cell_area; mod get_resolution; @@ -8,7 +9,8 @@ criterion_group!( benches, get_resolution::bench, // get_cell_area::bench, - encode_point::bench + encode_point::bench, + decode_point::bench ); criterion_main!(benches); diff --git a/cliff.toml b/cliff.toml index 74e75df..3e06ed3 100644 --- a/cliff.toml +++ b/cliff.toml @@ -62,16 +62,16 @@ commit_preprocessors = [ commit_parsers = [ { message = "^feat", group = "๐Ÿš€ Features" }, { message = "^fix", group = "๐Ÿ› Bug Fixes" }, - { message = "^doc", group = "๐Ÿ“š Documentation", skip = true}, + { message = "^doc", group = "๐Ÿ“š Documentation" }, { message = "^perf", group = "โšก Performance" }, - { message = "^refactor", group = "๐Ÿšœ Refactor", skip = true}, + { message = "^refactor", group = "๐Ÿšœ Refactor" }, { message = "^style", group = "๐ŸŽจ Styling" }, - { message = "^test", group = "๐Ÿงช Testing", skip = true}, - { message = "^chore\\(release\\): prepare for", skip = true }, - { message = "^chore\\(deps.*\\)", skip = true }, - { message = "^chore\\(pr\\)", skip = true }, - { message = "^chore\\(pull\\)", skip = true }, - { message = "^chore|^ci", group = "โš™๏ธ Miscellaneous Tasks" , skip = true}, + { message = "^test", group = "๐Ÿงช Testing" }, + { message = "^chore\\(release\\): prepare for" }, + { message = "^chore\\(deps.*\\)" }, + { message = "^chore\\(pr\\)" }, + { message = "^chore\\(pull\\)" }, + { message = "^chore|^ci", group = "โš™๏ธ Miscellaneous Tasks" }, { body = ".*security", group = "๐Ÿ›ก๏ธ Security" }, { message = "^revert", group = "โ—€๏ธ Revert" }, { message = ".*", group = "๐Ÿ’ผ Other" }, From 433ee84519c196a1e6cae0253b5d41ed52160f58 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:56:43 +1200 Subject: [PATCH 07/13] fmt: cargo formatting --- src/cells.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cells.rs b/src/cells.rs index 90c612b..dc6316a 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -56,7 +56,7 @@ impl Cell { /// # Example /// ``` /// use qbin::Cell; - /// + /// /// let cell_new = Cell::new(5234261499580514303); /// let cell_try = Cell::try_from(5234261499580514303).expect("cell index"); /// assert_eq!(cell_new, cell_try); @@ -89,8 +89,6 @@ impl Cell { /// let parent = qb_cell.parent(2_u8).expect("cell index"); /// assert_eq!(parent, Cell::try_from(5200813144682790911).expect("cell index")) /// ``` - // TODO: - // Return Option as in neighbor pub fn parent(&self, parent_res: u8) -> Result { cell_to_parent(self, parent_res) } From c0bf963f16212a1255770249ff0a2d39bbbbaabd Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 15:59:04 +1200 Subject: [PATCH 08/13] doc: README update --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d5b4654..06b60b3 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ let qb = Cell::from_point(latitude, longitude, resolution).expect("cell index"); assert_eq!(qb, Cell::try_from(5234261499580514303_u64).expect("cell index")); // Get a point from a Quadbin cell -let coords = Cell::try_from(5209574053332910079_u64).expect("cell index").to_point(); +let coords = Cell::new(5209574053332910079_u64).to_point(); assert_eq!(coords, [-11.178401873711776, 33.75]); // Quadbin resolution at equator in mยฒ From 5a959cc92040fc010675132134454e6fe29fe83c Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 16:14:45 +1200 Subject: [PATCH 09/13] refactor: better error handling --- src/cells.rs | 12 +++++++----- src/test/cells.rs | 5 +++-- src/utils.rs | 6 ++++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/cells.rs b/src/cells.rs index dc6316a..aaa385a 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -116,7 +116,7 @@ impl Cell { /// ``` pub fn neighbor(&self, direction: Direction) -> Option { let tile = self.to_tile().neighbor(direction)?; - Some(tile.to_cell().expect("cell index")) + tile.to_cell().ok() } /// Find the Cell's sibling in a specific [Direction]. @@ -348,10 +348,12 @@ fn cell_to_point(cell: &Cell) -> [f64; 2] { fn cell_to_parent(cell: &Cell, parent_res: u8) -> Result { // Check resolution let resolution = cell.resolution(); - assert!( - parent_res < resolution, - "parent resolution should be lower than current resolution" - ); + if parent_res >= resolution { + return Err(InvalidCell::new( + Some(cell.get()), + "Parent resolution should be lower than the current resolution", + )); + } let result = (cell.get() & !(0x1F << 52)) | ((parent_res as u64) << 52) diff --git a/src/test/cells.rs b/src/test/cells.rs index 54f113d..79321e7 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -131,10 +131,11 @@ fn test_cell_to_parent() { } } #[test] -#[should_panic(expected = "parent resolution should be lower than current resolution")] fn test_cell_to_parent_invalid_resolution() { let cell = Cell::try_from(5209574053332910079).expect("cell index"); - let _ = cell.parent(4); + let result = cell.parent(4); + + assert!(result.is_err()); } // Estimate cell area diff --git a/src/utils.rs b/src/utils.rs index 31d4cc1..93e68c3 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -22,6 +22,8 @@ pub(crate) fn clip_latitude(lat: f64) -> f64 { /// specific resolution. pub(crate) fn point_to_tile_fraction(lat: f64, lng: f64, res: u8) -> (f64, f64, u8) { // Check resolution to avoid overflow + // TODO: + // Replace with Error assert!( (res <= MAX_RESOLUTION), "Resolution should be between 0 and 26" @@ -52,6 +54,8 @@ pub(crate) fn point_to_tile(lat: f64, lng: f64, res: u8) -> Tile { /// Compute the latitude for a tile with an offset. pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> f64 { // Check if offset is between 0 and 1 + // TODO: + // Replace with Error assert!( (0.0..=1.0).contains(&offset), "Offset should be between 0 and 1" @@ -69,6 +73,8 @@ pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> f64 { /// Compute the longitude for a tile with an offset. pub(crate) fn tile_to_longitude(tile: &Tile, offset: f64) -> f64 { // Check if offset is between 0 and 1 + // TODO: + // Replace with Error assert!( (0.0..=1.0).contains(&offset), "Offset should be between 0 and 1" From 59c6807f2abdf0006ca6773117fc8e15c0820417 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 16:40:35 +1200 Subject: [PATCH 10/13] chore: replace all asserts with Error --- src/cells.rs | 14 ++++++------ src/errors.rs | 2 ++ src/test/tiles.rs | 11 +++++----- src/tiles.rs | 8 +++---- src/utils.rs | 56 ++++++++++++++++++++++++++--------------------- 5 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/cells.rs b/src/cells.rs index aaa385a..5739259 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -209,12 +209,12 @@ impl Cell { pub fn to_bbox(&self) -> [f64; 4] { let tile = &self.to_tile(); - let xmin = &tile.to_longitude(0.0); - let xmax = &tile.to_longitude(1.0); - let ymin = &tile.to_latitude(1.0); - let ymax = &tile.to_latitude(0.0); + let xmin = tile.to_longitude(0.0).expect("offset"); + let xmax = tile.to_longitude(1.0).expect("offset"); + let ymin = tile.to_latitude(1.0).expect("offset"); + let ymax = tile.to_latitude(0.0).expect("offset"); - [*xmin, *ymin, *xmax, *ymax] + [xmin, ymin, xmax, ymax] } /// Convert a geographic point into a Quadbin cell. @@ -336,8 +336,8 @@ fn point_to_cell(lat: f64, lng: f64, res: u8) -> Result { /// Convert cell into point fn cell_to_point(cell: &Cell) -> [f64; 2] { let tile = cell.to_tile(); - let lat = tile.to_latitude(0.5); - let lon = tile.to_longitude(0.5); + let lat = tile.to_latitude(0.5).expect("offset"); + let lon = tile.to_longitude(0.5).expect("offset"); // Return array, not tuple, as it more memory efficient // See https://doc.rust-lang.org/stable/book/ch03-02-data-types.html#the-array-type diff --git a/src/errors.rs b/src/errors.rs index 1a1e606..e190e83 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -46,3 +46,5 @@ macro_rules! invalid_value_error { invalid_value_error!("direction", InvalidDirection, u8); invalid_value_error!("cell index", InvalidCell, Option); +invalid_value_error!("resolution", InvalidResolution, u8); +invalid_value_error!("offset", InvalidOffset, f64); diff --git a/src/test/tiles.rs b/src/test/tiles.rs index ab8360a..adbb49e 100644 --- a/src/test/tiles.rs +++ b/src/test/tiles.rs @@ -10,7 +10,8 @@ const ACC: f64 = 1e-10; // See https://github.com/CartoDB/quadbin-py/blob/master/tests/unit/test_utils.py #[test] fn test_point_to_tile_fraction() { - let tile = point_to_tile_fraction(41.26000108568697_f64, -95.93965530395508_f64, 9_u8); + let tile = point_to_tile_fraction(41.26000108568697_f64, -95.93965530395508_f64, 9_u8) + .expect("resolution"); assert_relative_eq!(tile.0, 119.552490234375_f64, epsilon = ACC); assert_relative_eq!(tile.1, 191.47119140625_f64, epsilon = ACC); assert_eq!(tile.2, 9_u8); @@ -69,16 +70,16 @@ fn test_tile_conversion() { assert_eq!(tile.z, 10_u8); // Convert back to coordinates - let new_lon = tile.to_longitude(0.0); - let new_lat = tile.to_latitude(0.0); + let new_lon = tile.to_longitude(0.0).expect("offset"); + let new_lat = tile.to_latitude(0.0).expect("offset"); // Check conversion with approximate equality assert_relative_eq!(new_lat, 45.08903556483104_f64, epsilon = ACC); assert_relative_eq!(new_lon, lon, epsilon = ACC); // Check offset with approximate equality - let new_lon_offset = tile.to_longitude(0.5); - let new_lat_offset = tile.to_latitude(0.5); + let new_lon_offset = tile.to_longitude(0.5).expect("offset"); + let new_lat_offset = tile.to_latitude(0.5).expect("offset"); assert_relative_eq!(new_lat_offset, 44.96479793033102_f64, epsilon = ACC); assert_relative_eq!(new_lon_offset, -44.82421875_f64, epsilon = ACC); } diff --git a/src/tiles.rs b/src/tiles.rs index a5de61a..b1c72cf 100644 --- a/src/tiles.rs +++ b/src/tiles.rs @@ -38,16 +38,16 @@ impl Tile { /// /// See also [Tile::to_longitude]. /// - pub fn to_latitude(self, offset: f64) -> f64 { - tile_to_latitude(&self, offset) + pub fn to_latitude(self, offset: f64) -> Option { + tile_to_latitude(&self, offset).ok() } /// Return tile's longitude. /// /// See also [Tile::to_latitude]. /// - pub fn to_longitude(self, offset: f64) -> f64 { - tile_to_longitude(&self, offset) + pub fn to_longitude(self, offset: f64) -> Option { + tile_to_longitude(&self, offset).ok() } /// Get tile's siblings. diff --git a/src/utils.rs b/src/utils.rs index 93e68c3..af05ef8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,5 +1,6 @@ use crate::constants::*; use crate::directions::Direction; +use crate::errors::{InvalidOffset, InvalidResolution}; use crate::tiles::Tile; use std::f64::consts::PI; @@ -20,14 +21,18 @@ pub(crate) fn clip_latitude(lat: f64) -> f64 { /// Compute the tile in fractions for a longitude and latitude in a /// specific resolution. -pub(crate) fn point_to_tile_fraction(lat: f64, lng: f64, res: u8) -> (f64, f64, u8) { +pub(crate) fn point_to_tile_fraction( + lat: f64, + lng: f64, + res: u8, +) -> Result<(f64, f64, u8), InvalidResolution> { // Check resolution to avoid overflow - // TODO: - // Replace with Error - assert!( - (res <= MAX_RESOLUTION), - "Resolution should be between 0 and 26" - ); + if res > MAX_RESOLUTION { + return Err(InvalidResolution::new( + res, + "Resolution should be between 0 and 26", + )); + } // Compute tile coordinates let z2: f64 = (1 << res) as f64; @@ -40,26 +45,26 @@ pub(crate) fn point_to_tile_fraction(lat: f64, lng: f64, res: u8) -> (f64, f64, let x = if x < 0.0 { x + z2 } else { x }; // Return the tile coordinates - (x, y, res) + Ok((x, y, res)) } /// Compute the tile for a longitude and latitude in a specific resolution. pub(crate) fn point_to_tile(lat: f64, lng: f64, res: u8) -> Tile { - let (x, y, z) = point_to_tile_fraction(lat, lng, res); + let (x, y, z) = point_to_tile_fraction(lat, lng, res).expect("resolution"); let x: u32 = x.floor() as u32; let y: u32 = y.floor() as u32; Tile::new(x, y, z) } /// Compute the latitude for a tile with an offset. -pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> f64 { +pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> Result { // Check if offset is between 0 and 1 - // TODO: - // Replace with Error - assert!( - (0.0..=1.0).contains(&offset), - "Offset should be between 0 and 1" - ); + if !(0.0..=1.0).contains(&offset) { + return Err(InvalidOffset::new( + offset, + "Offset should be between 0.0 and 1.0", + )); + } // Get Tile coords let y = tile.y as f64; @@ -67,25 +72,26 @@ pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> f64 { // Compute latitude let expy = f64::exp(-(2.0 * (y + offset) / z2 - 1.0) * PI); - 360.0 * (f64::atan(expy) / PI - 0.25) + let lat = 360.0 * (f64::atan(expy) / PI - 0.25); + Ok(lat) } /// Compute the longitude for a tile with an offset. -pub(crate) fn tile_to_longitude(tile: &Tile, offset: f64) -> f64 { +pub(crate) fn tile_to_longitude(tile: &Tile, offset: f64) -> Result { // Check if offset is between 0 and 1 - // TODO: - // Replace with Error - assert!( - (0.0..=1.0).contains(&offset), - "Offset should be between 0 and 1" - ); + if !(0.0..=1.0).contains(&offset) { + return Err(InvalidOffset::new( + offset, + "Offset should be between 0.0 and 1.0", + )); + } // Get Tile coords let x = tile.x as f64; let z2 = (1 << tile.z) as f64; // Compute longitude - 180.0 * (2.0 * (x + offset) / z2 - 1.0) + Ok(180.0 * (2.0 * (x + offset) / z2 - 1.0)) } /// Inverse of the scale factor at the tile center. From 3677ba0c79bb80914f1eb31af5a5f765037c44d8 Mon Sep 17 00:00:00 2001 From: Anatolii Tsyplenkov Date: Tue, 29 Apr 2025 16:42:31 +1200 Subject: [PATCH 11/13] todo --- src/errors.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/errors.rs b/src/errors.rs index e190e83..2b94731 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -48,3 +48,6 @@ invalid_value_error!("direction", InvalidDirection, u8); invalid_value_error!("cell index", InvalidCell, Option); invalid_value_error!("resolution", InvalidResolution, u8); invalid_value_error!("offset", InvalidOffset, f64); + +// TODO: +// Add Error enum From a31e2a7ecf9600b625a9ea720fc4a7b450ad97d0 Mon Sep 17 00:00:00 2001 From: Anatoly Tsyplenkov Date: Tue, 29 Apr 2025 20:36:41 +1200 Subject: [PATCH 12/13] chore: added QuadbinError enum (closes #11) --- src/cells.rs | 26 +++++++++++++------------- src/errors.rs | 21 +++++++++++++++++++-- src/test/cells.rs | 29 ++--------------------------- src/test/errors.rs | 37 +++++++++++++++++++++++++++++++++++++ src/test/mod.rs | 1 + src/tiles.rs | 4 ++-- src/utils.rs | 20 ++++++++++---------- 7 files changed, 84 insertions(+), 54 deletions(-) create mode 100644 src/test/errors.rs diff --git a/src/cells.rs b/src/cells.rs index 5739259..2a7cae7 100644 --- a/src/cells.rs +++ b/src/cells.rs @@ -1,7 +1,7 @@ use crate::Direction; use crate::constants::*; use crate::errors; -use crate::errors::InvalidCell; +use crate::errors::*; use crate::tiles::*; use crate::utils::*; use core::{fmt, num::NonZeroU64}; @@ -29,14 +29,14 @@ use core::{fmt, num::NonZeroU64}; pub struct Cell(NonZeroU64); impl TryFrom for Cell { - type Error = errors::InvalidCell; + type Error = errors::QuadbinError; - fn try_from(value: u64) -> Result { + fn try_from(value: u64) -> Result { if !is_valid_cell(value) { - return Err(Self::Error::new( + return Err(QuadbinError::InvalidCell(InvalidCell::new( Some(value), "Provided Quadbin Cell index is invalid", - )); + ))); } Ok(Self(NonZeroU64::new(value).expect("non-zero cell index"))) @@ -89,7 +89,7 @@ impl Cell { /// let parent = qb_cell.parent(2_u8).expect("cell index"); /// assert_eq!(parent, Cell::try_from(5200813144682790911).expect("cell index")) /// ``` - pub fn parent(&self, parent_res: u8) -> Result { + pub fn parent(&self, parent_res: u8) -> Result { cell_to_parent(self, parent_res) } @@ -227,7 +227,7 @@ impl Cell { /// let cell = Cell::from_point(-41.28303675124842, 174.77727344223067, 26).expect("cell index"); /// assert_eq!(cell.get(), 5309133744805926483_u64) /// ``` - pub fn from_point(lat: f64, lng: f64, res: u8) -> Result { + pub fn from_point(lat: f64, lng: f64, res: u8) -> Result { point_to_cell(lat, lng, res) } @@ -264,7 +264,7 @@ fn is_valid_cell(cell64: u64) -> bool { } /// Convert a tile into a Quadbin cell. -pub(crate) fn tile_to_cell(tile: Tile) -> Result { +pub(crate) fn tile_to_cell(tile: Tile) -> Result { let mut x = tile.x as u64; let mut y = tile.y as u64; let z = tile.z as u64; @@ -324,7 +324,7 @@ fn cell_to_tile(cell: &Cell) -> Tile { } /// Convert a geographic point into a cell. -fn point_to_cell(lat: f64, lng: f64, res: u8) -> Result { +fn point_to_cell(lat: f64, lng: f64, res: u8) -> Result { let lng = clip_longitude(lng); let lat = clip_latitude(lat); @@ -345,14 +345,14 @@ fn cell_to_point(cell: &Cell) -> [f64; 2] { } /// Compute the parent cell for a specific resolution. -fn cell_to_parent(cell: &Cell, parent_res: u8) -> Result { +fn cell_to_parent(cell: &Cell, parent_res: u8) -> Result { // Check resolution let resolution = cell.resolution(); if parent_res >= resolution { - return Err(InvalidCell::new( - Some(cell.get()), + return Err(QuadbinError::InvalidResolution(InvalidResolution::new( + parent_res, "Parent resolution should be lower than the current resolution", - )); + ))); } let result = (cell.get() & !(0x1F << 52)) diff --git a/src/errors.rs b/src/errors.rs index 2b94731..29fd8c6 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -49,5 +49,22 @@ invalid_value_error!("cell index", InvalidCell, Option); invalid_value_error!("resolution", InvalidResolution, u8); invalid_value_error!("offset", InvalidOffset, f64); -// TODO: -// Add Error enum +// One enum to rule them all +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum QuadbinError { + InvalidResolution(InvalidResolution), + InvalidOffset(InvalidOffset), + InvalidCell(InvalidCell), +} + +impl std::fmt::Display for QuadbinError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + QuadbinError::InvalidResolution(e) => write!(f, "{}", e), + QuadbinError::InvalidOffset(e) => write!(f, "{}", e), + QuadbinError::InvalidCell(e) => write!(f, "{}", e), + } + } +} + +impl std::error::Error for QuadbinError {} diff --git a/src/test/cells.rs b/src/test/cells.rs index 79321e7..0301c14 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -1,6 +1,5 @@ use crate::cells::*; use crate::directions::Direction; -use crate::errors::*; use crate::tiles::*; use approx::assert_relative_eq; @@ -124,12 +123,10 @@ fn test_cell_to_parent() { ]; for (cell, res, parent) in cases.iter() { - assert_eq!( - Cell::try_from(*cell).expect("cell index").parent(*res), - Cell::try_from(*parent) - ); + assert_eq!(Cell::new(*cell).parent(*res).unwrap(), Cell::new(*parent)); } } + #[test] fn test_cell_to_parent_invalid_resolution() { let cell = Cell::try_from(5209574053332910079).expect("cell index"); @@ -252,25 +249,3 @@ fn test_cell_neighbors() { .neighbors(); assert_eq!(nn[1], None) } - -#[test] -fn test_invalid_cellindex() { - assert!( - !InvalidCell::new(Some(5209574053332910078_u64), "error") - .to_string() - .is_empty() - ); - assert_eq!( - Cell::try_from(5209574053332910078_u64).err(), - Some(InvalidCell::new( - Some(5209574053332910078_u64), - "Provided Quadbin Cell index is invalid" - )) - ); - assert_eq!( - Cell::try_from(5209574053332910079_u64) - .expect("cell index") - .get(), - 5209574053332910079_u64 - ); -} diff --git a/src/test/errors.rs b/src/test/errors.rs new file mode 100644 index 0000000..63fefdb --- /dev/null +++ b/src/test/errors.rs @@ -0,0 +1,37 @@ +use crate::Cell; +use crate::errors::*; + +#[test] +fn test_invalid_cellindex() { + assert!( + !InvalidCell::new(Some(5209574053332910078_u64), "error") + .to_string() + .is_empty() + ); + assert_eq!( + Cell::try_from(5209574053332910078_u64).err(), + Some(QuadbinError::InvalidCell(InvalidCell::new( + Some(5209574053332910078_u64), + "Provided Quadbin Cell index is invalid" + ))) + ); + assert_eq!( + Cell::try_from(5209574053332910079_u64) + .expect("cell index") + .get(), + 5209574053332910079_u64 + ); +} + +#[test] +fn test_parent_resolution() { + let cell = Cell::new(5209574053332910079); + let message = cell.parent(26).err(); + assert_eq!( + message, + Some(QuadbinError::InvalidResolution(InvalidResolution::new( + 26, + "Parent resolution should be lower than the current resolution" + ))) + ); +} diff --git a/src/test/mod.rs b/src/test/mod.rs index 191cc0d..c299d71 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -1,4 +1,5 @@ mod cells; mod directions; +mod errors; mod hashing; mod tiles; diff --git a/src/tiles.rs b/src/tiles.rs index b1c72cf..ca43845 100644 --- a/src/tiles.rs +++ b/src/tiles.rs @@ -1,6 +1,6 @@ use crate::Direction; use crate::cells::*; -use crate::errors::InvalidCell; +use crate::errors::QuadbinError; use crate::utils::*; /// A single tile coordinates @@ -20,7 +20,7 @@ impl Tile { } /// Convert to Quadbin cell. - pub fn to_cell(self) -> Result { + pub fn to_cell(self) -> Result { tile_to_cell(self) } diff --git a/src/utils.rs b/src/utils.rs index af05ef8..3eab058 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,6 +1,6 @@ use crate::constants::*; use crate::directions::Direction; -use crate::errors::{InvalidOffset, InvalidResolution}; +use crate::errors::{InvalidOffset, InvalidResolution, QuadbinError}; use crate::tiles::Tile; use std::f64::consts::PI; @@ -25,13 +25,13 @@ pub(crate) fn point_to_tile_fraction( lat: f64, lng: f64, res: u8, -) -> Result<(f64, f64, u8), InvalidResolution> { +) -> Result<(f64, f64, u8), QuadbinError> { // Check resolution to avoid overflow if res > MAX_RESOLUTION { - return Err(InvalidResolution::new( + return Err(QuadbinError::InvalidResolution(InvalidResolution::new( res, "Resolution should be between 0 and 26", - )); + ))); } // Compute tile coordinates @@ -57,13 +57,13 @@ pub(crate) fn point_to_tile(lat: f64, lng: f64, res: u8) -> Tile { } /// Compute the latitude for a tile with an offset. -pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> Result { +pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> Result { // Check if offset is between 0 and 1 if !(0.0..=1.0).contains(&offset) { - return Err(InvalidOffset::new( + return Err(QuadbinError::InvalidOffset(InvalidOffset::new( offset, "Offset should be between 0.0 and 1.0", - )); + ))); } // Get Tile coords @@ -77,13 +77,13 @@ pub(crate) fn tile_to_latitude(tile: &Tile, offset: f64) -> Result Result { +pub(crate) fn tile_to_longitude(tile: &Tile, offset: f64) -> Result { // Check if offset is between 0 and 1 if !(0.0..=1.0).contains(&offset) { - return Err(InvalidOffset::new( + return Err(QuadbinError::InvalidOffset(InvalidOffset::new( offset, "Offset should be between 0.0 and 1.0", - )); + ))); } // Get Tile coords From 67b2fbab1bd05e60d6a2034ea651f0a7464be114 Mon Sep 17 00:00:00 2001 From: Anatoly Tsyplenkov Date: Tue, 29 Apr 2025 20:47:03 +1200 Subject: [PATCH 13/13] test: better error tests --- src/test/cells.rs | 8 -------- src/test/errors.rs | 31 ++++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/test/cells.rs b/src/test/cells.rs index 0301c14..fa6f54d 100644 --- a/src/test/cells.rs +++ b/src/test/cells.rs @@ -127,14 +127,6 @@ fn test_cell_to_parent() { } } -#[test] -fn test_cell_to_parent_invalid_resolution() { - let cell = Cell::try_from(5209574053332910079).expect("cell index"); - let result = cell.parent(4); - - assert!(result.is_err()); -} - // Estimate cell area #[test] fn test_cell_area() { diff --git a/src/test/errors.rs b/src/test/errors.rs index 63fefdb..625efb4 100644 --- a/src/test/errors.rs +++ b/src/test/errors.rs @@ -24,14 +24,35 @@ fn test_invalid_cellindex() { } #[test] -fn test_parent_resolution() { - let cell = Cell::new(5209574053332910079); - let message = cell.parent(26).err(); +fn test_cell_to_parent_invalid_resolution() { + let cell = Cell::try_from(5209574053332910079).expect("cell index"); + let result = cell.parent(4); + + assert!(result.is_err()); + assert_eq!( - message, + result.err(), Some(QuadbinError::InvalidResolution(InvalidResolution::new( - 26, + 4, "Parent resolution should be lower than the current resolution" ))) ); } + +#[test] +fn test_invalid_cell_index() { + let val: [u64; 2] = [5209574053332910078, 6362495557939757055]; + + for i in val.iter() { + let cell = Cell::try_from(*i); + assert!(cell.is_err()); + + assert_eq!( + cell.err(), + Some(QuadbinError::InvalidCell(InvalidCell::new( + Some(*i), + "Provided Quadbin Cell index is invalid" + ))) + ); + } +}