Skip to content

Commit 746afed

Browse files
committed
Fix smspec_node operators
The operators would throw a TypeError before this fix, but now they follow the usual conventions of returning NotImplemented.
1 parent 6042ed5 commit 746afed

3 files changed

Lines changed: 58 additions & 20 deletions

File tree

lib/include/resdata/smspec_node.hpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,16 @@ class smspec_node {
117117
return node1->cmp(*node2);
118118
}
119119

120+
bool operator==(const smspec_node &other) const {
121+
return this->cmp(other) == 0;
122+
}
123+
bool operator<(const smspec_node &other) const {
124+
return this->cmp(other) < 0;
125+
}
126+
bool operator>(const smspec_node &other) const {
127+
return this->cmp(other) > 0;
128+
}
129+
120130
int get_R1() const;
121131
int get_R2() const;
122132
const char *get_gen_key1() const;

lib/resdata/smspec_node_pybind.cpp

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <pybind11/operators.h>
12
#include <pybind11/pybind11.h>
23
#include <pybind11/stl.h>
34

@@ -16,13 +17,6 @@ py::object SummaryVarType() {
1617
return cls;
1718
}
1819

19-
int node_cmp(const rd::smspec_node &self, py::handle other) {
20-
if (!py::isinstance<rd::smspec_node>(other))
21-
throw py::type_error(
22-
"Other argument must be of type ResdataSMSPECNode");
23-
return self.cmp(other.cast<const rd::smspec_node &>());
24-
}
25-
2620
} // namespace
2721

2822
PYBIND11_MODULE(rd_smspec_node, m) {
@@ -32,19 +26,9 @@ PYBIND11_MODULE(rd_smspec_node, m) {
3226
"Class can not be instantiated directly!");
3327
throw py::error_already_set();
3428
}))
35-
.def("cmp", &node_cmp, py::arg("other"))
36-
.def("__lt__",
37-
[](const rd::smspec_node &self, py::handle other) {
38-
return node_cmp(self, other) < 0;
39-
})
40-
.def("__gt__",
41-
[](const rd::smspec_node &self, py::handle other) {
42-
return node_cmp(self, other) > 0;
43-
})
44-
.def("__eq__",
45-
[](const rd::smspec_node &self, py::handle other) {
46-
return node_cmp(self, other) == 0;
47-
})
29+
.def(py::self == py::self)
30+
.def(py::self < py::self)
31+
.def(py::self > py::self)
4832
.def("__hash__",
4933
[](const rd::smspec_node &self) {
5034
return py::hash(py::cast(self.get_gen_key1()));

tests/rd_tests/test_rd_sum.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,50 @@ def test_that_non_matching_dataframe_gives_empty_columns(summary):
280280
)
281281

282282

283+
@given(summaries())
284+
@pytest.mark.usefixtures("use_tmpdir")
285+
def test_that_equal_smspec_nodes_have_equal_hash(summary):
286+
"""The __hash__/__eq__ contract requires that a == b implies
287+
hash(a) == hash(b) for all smspec_nodes of a summary case."""
288+
smspec, unsmry = summary
289+
assume(len(smspec.keywords) == len(set(smspec.keywords)))
290+
smspec.to_file("TEST.SMSPEC")
291+
unsmry.to_file("TEST.UNSMRY")
292+
summary = Summary("TEST", lazy_load=False)
293+
294+
nodes = [summary.smspec_node(key) for key in summary.keys()]
295+
for node1, node2 in itertools.product(nodes, repeat=2):
296+
if node1 == node2:
297+
assert hash(node1) == hash(node2)
298+
299+
300+
@given(summaries())
301+
@pytest.mark.usefixtures("use_tmpdir")
302+
def test_that_smspec_node_comparison_with_other_types_is_consistent(summary):
303+
"""Comparing a smspec_node to an object of another type should never
304+
consider them equal, and ordering comparisons should raise TypeError,
305+
matching normal Python semantics for unsupported comparisons."""
306+
smspec, unsmry = summary
307+
assume(len(smspec.keywords) == len(set(smspec.keywords)))
308+
assume(len(smspec.keywords) > 0)
309+
smspec.to_file("TEST.SMSPEC")
310+
unsmry.to_file("TEST.UNSMRY")
311+
summary = Summary("TEST", lazy_load=False)
312+
313+
node = summary.smspec_node(next(iter(summary.keys())))
314+
315+
assert node != "a_string"
316+
assert not (node == 5)
317+
assert node is not None
318+
assert node != None # noqa: E711
319+
320+
with pytest.raises(TypeError):
321+
node < "a_string"
322+
323+
with pytest.raises(TypeError):
324+
node > 5
325+
326+
283327
def create_summary(
284328
summary_keys=("FOPR",),
285329
time_units="DAYS",

0 commit comments

Comments
 (0)