Skip to content

Commit

Permalink
Use FromStr and std::fmt::Display (e.g. ToString) instead of From
Browse files Browse the repository at this point in the history
  • Loading branch information
westonpace committed Aug 5, 2024
1 parent b1b68e6 commit 7d1f114
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 51 deletions.
8 changes: 1 addition & 7 deletions python/src/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ use lance::dataset::{ColumnAlteration, ProjectionRequest};
use lance::index::{vector::VectorIndexParams, DatasetIndexInternalExt};
use lance_arrow::as_fixed_size_list_array;
use lance_core::datatypes::Schema;
use lance_file::version::LanceFileVersion;
use lance_index::optimize::OptimizeOptions;
use lance_index::scalar::FullTextSearchQuery;
use lance_index::vector::hnsw::builder::HnswBuildParams;
use lance_index::vector::sq::builder::SQBuildParams;
use lance_index::{
optimize::OptimizeOptions,
scalar::{FullTextSearchQuery, ScalarIndexParams, ScalarIndexType},
Expand Down Expand Up @@ -1378,8 +1373,7 @@ pub fn get_write_params(options: &PyDict) -> PyResult<Option<WriteParams>> {
}
if let Some(data_storage_version) = get_dict_opt::<String>(options, "data_storage_version")?
{
p.data_storage_version =
Some(LanceFileVersion::try_from(data_storage_version).infer_error()?);
p.data_storage_version = Some(data_storage_version.parse().infer_error()?);
}
if let Some(progress) = get_dict_opt::<PyObject>(options, "progress")? {
p.progress = Arc::new(PyWriteProgress::new(progress.to_object(options.py())));
Expand Down
2 changes: 1 addition & 1 deletion python/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl LanceFileWriter {
data_cache_bytes,
keep_original_array,
format_version: version
.map(|v| LanceFileVersion::try_from(v.as_str()))
.map(|v| v.parse::<LanceFileVersion>())
.transpose()
.infer_error()?,
..Default::default()
Expand Down
40 changes: 16 additions & 24 deletions rust/lance-encoding/src/version.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: Copyright The Lance Authors

use std::str::FromStr;

use lance_core::{Error, Result};
use snafu::{location, Location};

Expand Down Expand Up @@ -43,26 +45,24 @@ impl LanceFileVersion {

impl std::fmt::Display for LanceFileVersion {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", String::from(*self))
}
}

impl From<LanceFileVersion> for String {
fn from(version: LanceFileVersion) -> Self {
match version {
LanceFileVersion::Legacy => LEGACY_FORMAT_VERSION.to_string(),
LanceFileVersion::V2_0 => V2_FORMAT_2_0.to_string(),
LanceFileVersion::V2_1 => V2_FORMAT_2_1.to_string(),
LanceFileVersion::Stable => "stable".to_string(),
LanceFileVersion::Next => "next".to_string(),
}
write!(
f,
"{}",
match self {
Self::Legacy => LEGACY_FORMAT_VERSION,
Self::V2_0 => V2_FORMAT_2_0,
Self::V2_1 => V2_FORMAT_2_1,
Self::Stable => "stable",
Self::Next => "next",
}
)
}
}

impl TryFrom<&str> for LanceFileVersion {
type Error = Error;
impl FromStr for LanceFileVersion {
type Err = Error;

fn try_from(value: &str) -> Result<Self> {
fn from_str(value: &str) -> Result<Self> {
match value.to_lowercase().as_str() {
LEGACY_FORMAT_VERSION => Ok(Self::Legacy),
V2_FORMAT_2_0 => Ok(Self::V2_0),
Expand All @@ -78,11 +78,3 @@ impl TryFrom<&str> for LanceFileVersion {
}
}
}

impl TryFrom<String> for LanceFileVersion {
type Error = Error;

fn try_from(value: String) -> Result<Self> {
Self::try_from(value.as_str())
}
}
7 changes: 5 additions & 2 deletions rust/lance-table/src/format/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,15 @@ const LANCE_FORMAT_NAME: &str = "lance";

impl DataStorageFormat {
pub fn new(version: LanceFileVersion) -> Self {
let version_str = String::from(version.resolve());
Self {
file_format: LANCE_FORMAT_NAME.to_string(),
version: version_str,
version: version.resolve().to_string(),
}
}

pub fn lance_file_version(&self) -> Result<LanceFileVersion> {
self.version.parse::<LanceFileVersion>()
}
}

impl Default for DataStorageFormat {
Expand Down
5 changes: 2 additions & 3 deletions rust/lance/src/dataset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use lance_core::{datatypes::SchemaCompareOptions, traits::DatasetTakeRows};
use lance_datafusion::projection::ProjectionPlan;
use lance_datafusion::utils::{peek_reader_schema, reader_to_stream};
use lance_file::datatypes::populate_schema_dictionary;
use lance_file::version::LanceFileVersion;
use lance_io::object_store::{ObjectStore, ObjectStoreParams, ObjectStoreRegistry};
use lance_io::object_writer::ObjectWriter;
use lance_io::traits::WriteExt;
Expand Down Expand Up @@ -527,8 +526,7 @@ impl Dataset {
},
)?;
// If appending, always use existing storage version
storage_version =
LanceFileVersion::try_from(m.data_storage_format.version.as_str())?;
storage_version = m.data_storage_format.lance_file_version()?;
}
}

Expand Down Expand Up @@ -1507,6 +1505,7 @@ mod tests {
};
use lance_arrow::bfloat16::{self, ARROW_EXT_META_KEY, ARROW_EXT_NAME_KEY, BFLOAT16_EXT_NAME};
use lance_datagen::{array, gen, BatchCount, RowCount};
use lance_file::version::LanceFileVersion;
use lance_index::{scalar::ScalarIndexParams, vector::DIST_COL, DatasetIndexExt, IndexType};
use lance_linalg::distance::MetricType;
use lance_table::feature_flags;
Expand Down
13 changes: 5 additions & 8 deletions rust/lance/src/dataset/updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use arrow_array::{RecordBatch, UInt32Array};
use futures::StreamExt;
use lance_core::utils::deletion::DeletionVector;
use lance_core::{datatypes::Schema, Error, Result};
use lance_file::version::LanceFileVersion;
use lance_table::format::Fragment;
use lance_table::utils::stream::ReadBatchFutStream;
use snafu::{location, Location};
Expand Down Expand Up @@ -126,13 +125,11 @@ impl Updater {
///
/// Internal use only.
async fn new_writer(&mut self, schema: Schema) -> Result<Box<dyn GenericWriter>> {
let data_storage_version = LanceFileVersion::try_from(
self.dataset()
.manifest()
.data_storage_format
.version
.as_str(),
)?;
let data_storage_version = self
.dataset()
.manifest()
.data_storage_format
.lance_file_version()?;

open_writer(
&self.fragment.dataset().object_store,
Expand Down
7 changes: 4 additions & 3 deletions rust/lance/src/dataset/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,10 @@ pub async fn write_fragments_internal(
// Append mode, so we need to check compatibility
schema.check_compatible(dataset.schema(), &Default::default())?;
// If appending use storage version from dataset
storage_version = LanceFileVersion::try_from(
dataset.manifest().data_storage_format.version.as_str(),
)?;
storage_version = dataset
.manifest()
.data_storage_format
.lance_file_version()?;
// Use the schema from the dataset, because it has the correct
// field ids.
dataset.schema()
Expand Down
7 changes: 4 additions & 3 deletions rust/lance/src/dataset/write/merge_insert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,10 @@ impl MergeInsertJob {
// Also, because we already sorted by row address, the rows
// will be in the correct order.

let data_storage_version = LanceFileVersion::try_from(
dataset.manifest().data_storage_format.version.as_str(),
)?;
let data_storage_version = dataset
.manifest()
.data_storage_format
.lance_file_version()?;
let mut writer = open_writer(
dataset.object_store(),
&write_schema,
Expand Down

0 comments on commit 7d1f114

Please sign in to comment.