Skip to content

Commit 4590f7f

Browse files
authored
Add meaningful equality comparisons to many rust wrappers (#11401)
### What pyo3 defaults `__eq__` to be equivalent to `is` but since we clone before returning most python objects that means `dataset.schema()==dataset.schema()` is False even though the contents are identical. I resolved this specific case and added a test. Then I asked a robot to write me a lint to enforce this everywhere, updated for things I could easily `derive(PartialEq, Eq)` then added NOLINT to the rest. The score is | With eq | Without eq | |---------|------------| | 8 | 15 |
1 parent 59a03b3 commit 4590f7f

18 files changed

+164
-34
lines changed

rerun_py/src/catalog/catalog_client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::catalog::{
1212
};
1313

1414
/// Client for a remote Rerun catalog server.
15-
#[pyclass(name = "CatalogClientInternal")]
15+
#[pyclass(name = "CatalogClientInternal")] // NOLINT: skip pyclass_eq, non-trivial implementation
1616
pub struct PyCatalogClientInternal {
1717
origin: re_uri::Origin,
1818

rerun_py/src/catalog/dataframe_query.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::catalog::{PyDatasetEntry, to_py_err};
2323
use crate::utils::{get_tokio_runtime, wait_for_future};
2424

2525
/// View into a remote dataset acting as DataFusion table provider.
26-
#[pyclass(name = "DataframeQueryView")]
26+
#[pyclass(name = "DataframeQueryView")] // NOLINT: skip pyclass_eq, non-trivial implementation
2727
pub struct PyDataframeQueryView {
2828
dataset: Py<PyDatasetEntry>,
2929

rerun_py/src/catalog/dataframe_rendering.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use pyo3::{Bound, PyAny, PyResult, pyclass, pymethods};
44

55
use re_format_arrow::{RecordBatchFormatOpts, format_record_batch_opts};
66

7-
#[pyclass(name = "RerunHtmlTable")]
8-
#[derive(Clone)]
7+
#[pyclass(eq, name = "RerunHtmlTable")]
8+
#[derive(Clone, PartialEq, Eq)]
99
pub struct PyRerunHtmlTable {
1010
max_width: Option<usize>,
1111
max_height: Option<usize>,

rerun_py/src/catalog/datafusion_table.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use tracing::instrument;
1010
use crate::catalog::PyCatalogClientInternal;
1111
use crate::utils::get_tokio_runtime;
1212

13-
#[pyclass(frozen, name = "DataFusionTable")]
13+
#[pyclass(frozen, name = "DataFusionTable")] // NOLINT: skip pyclass_eq, non-trivial implementation
1414
pub struct PyDataFusionTable {
1515
pub provider: Arc<dyn TableProvider + Send>,
1616
pub name: String,

rerun_py/src/catalog/dataset_entry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ use super::{
3737
};
3838

3939
/// A dataset entry in the catalog.
40-
#[pyclass(name = "DatasetEntry", extends=PyEntry)]
40+
#[pyclass(name = "DatasetEntry", extends=PyEntry)] // NOLINT: skip pyclass_eq, non-trivial implementation
4141
pub struct PyDatasetEntry {
4242
pub dataset_details: DatasetDetails,
4343
pub dataset_handle: DatasetHandle,

rerun_py/src/catalog/entry.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use re_protos::cloud::v1alpha1::{EntryKind, ext::EntryDetails};
88
use crate::catalog::PyCatalogClientInternal;
99

1010
/// A unique identifier for an entry in the catalog.
11-
#[pyclass(name = "EntryId")]
12-
#[derive(Clone)]
11+
#[pyclass(eq, name = "EntryId")]
12+
#[derive(Clone, PartialEq, Eq)]
1313
pub struct PyEntryId {
1414
pub id: EntryId,
1515
}
@@ -104,7 +104,7 @@ impl From<PyEntryKind> for EntryKind {
104104
// ---
105105

106106
/// An entry in the catalog.
107-
#[pyclass(name = "Entry", subclass)]
107+
#[pyclass(name = "Entry", subclass)] // NOLINT: skip pyclass_eq, non-trivial implementation
108108
pub struct PyEntry {
109109
pub client: Py<PyCatalogClientInternal>,
110110

rerun_py/src/catalog/table_entry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::{
2222
///
2323
/// Note: this object acts as a table provider for DataFusion.
2424
//TODO(ab): expose metadata about the table (e.g. stuff found in `provider_details`).
25-
#[pyclass(name = "TableEntry", extends=PyEntry)]
25+
#[pyclass(name = "TableEntry", extends=PyEntry)] // NOLINT: skip pyclass_eq, non-trivial implementation
2626
#[derive(Default)]
2727
pub struct PyTableEntry {
2828
lazy_provider: Option<Arc<dyn TableProvider + Send>>,

rerun_py/src/catalog/task.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use re_protos::common::v1alpha1::TaskId;
1111
use super::{PyCatalogClientInternal, PyDataFusionTable, to_py_err};
1212

1313
/// A handle on a remote task.
14-
#[pyclass(name = "Task")]
14+
#[pyclass(name = "Task")] // NOLINT: skip pyclass_eq, non-trivial implementation
1515
pub struct PyTask {
1616
pub client: Py<PyCatalogClientInternal>,
1717

@@ -48,7 +48,7 @@ impl PyTask {
4848

4949
/// A collection of [`Task`].
5050
#[allow(rustdoc::broken_intra_doc_links)]
51-
#[pyclass(name = "Tasks")]
51+
#[pyclass(name = "Tasks")] // NOLINT: skip pyclass_eq, non-trivial implementation
5252
pub struct PyTasks {
5353
client: Py<PyCatalogClientInternal>,
5454

rerun_py/src/dataframe/component_columns.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,11 @@ impl From<PyComponentColumnDescriptor> for ComponentColumnDescriptor {
119119
/// The component to select
120120
#[pyclass(
121121
frozen,
122+
eq,
122123
name = "ComponentColumnSelector",
123124
module = "rerun_bindings.rerun_bindings"
124125
)]
125-
#[derive(Clone)]
126+
#[derive(Clone, PartialEq, Eq)]
126127
pub struct PyComponentColumnSelector(pub ComponentColumnSelector);
127128

128129
#[pymethods]

rerun_py/src/dataframe/index_columns.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ use re_sorbet::{IndexColumnDescriptor, TimeColumnSelector};
1111
/// column, use [`IndexColumnSelector`][rerun.dataframe.IndexColumnSelector].
1212
#[pyclass(
1313
frozen,
14+
eq,
15+
hash,
1416
name = "IndexColumnDescriptor",
1517
module = "rerun_bindings.rerun_bindings"
1618
)]
17-
#[derive(Clone)]
19+
#[derive(Clone, PartialEq, Eq, Hash)]
1820
pub struct PyIndexColumnDescriptor(pub IndexColumnDescriptor);
1921

2022
#[pymethods]
@@ -56,10 +58,11 @@ impl From<IndexColumnDescriptor> for PyIndexColumnDescriptor {
5658
/// The name of the index to select. Usually the name of a timeline.
5759
#[pyclass(
5860
frozen,
61+
eq,
5962
name = "IndexColumnSelector",
6063
module = "rerun_bindings.rerun_bindings"
6164
)]
62-
#[derive(Clone)]
65+
#[derive(Clone, PartialEq, Eq)]
6366
pub struct PyIndexColumnSelector(pub TimeColumnSelector);
6467

6568
#[pymethods]

0 commit comments

Comments
 (0)