From 24d74d130f2b706215b0a680cb4fe4306b9dc9f1 Mon Sep 17 00:00:00 2001
From: Xavier Robin <xavier.robin@unibas.ch>
Date: Wed, 29 Mar 2023 15:10:46 +0200
Subject: [PATCH] fix: consistent behavior of GetHashCode/hash_code
Behavior was made more consistent. Methods are now available
on all the (Atom|Residue|Chain)(Handle|View)s, including in
Python, and documented. A hash code is an `unsigned long`,
and requesting it on an invalid handle raises an `InvalidHande`
exception.
In practice, the following was implemented:
- Exported to Python for ChainHandles
- Added method on ResidueViews and ChainViews (including Python export)
- Raise InvalidHandle error on ResidueHandle (was: return 0)
- Explicitly check validity of ChainHandle (although behavior was to
throw an error even without explicit check)
- Changed return value from `long` to `unsigned long` on
Atom(View|Handle)s.
---
modules/mol/base/doc/entity.rst | 60 ++++++++++++++++++-
modules/mol/base/pymod/export_chain.cc | 3 +
modules/mol/base/pymod/export_chain_view.cc | 3 +
modules/mol/base/pymod/export_residue_view.cc | 5 +-
modules/mol/base/src/atom_handle.cc | 4 +-
modules/mol/base/src/atom_handle.hh | 2 +-
modules/mol/base/src/atom_view.cc | 4 +-
modules/mol/base/src/atom_view.hh | 2 +-
modules/mol/base/src/bond_handle.cc | 1 +
modules/mol/base/src/chain_handle.cc | 1 +
modules/mol/base/src/chain_view.cc | 6 ++
modules/mol/base/src/chain_view.hh | 6 ++
modules/mol/base/src/residue_handle.cc | 1 +
modules/mol/base/src/residue_view.cc | 6 ++
modules/mol/base/src/residue_view.hh | 6 ++
modules/mol/base/tests/test_residue.cc | 29 +++++++++
16 files changed, 130 insertions(+), 9 deletions(-)
diff --git a/modules/mol/base/doc/entity.rst b/modules/mol/base/doc/entity.rst
index 173ea4a92..657125a2f 100644
--- a/modules/mol/base/doc/entity.rst
+++ b/modules/mol/base/doc/entity.rst
@@ -484,6 +484,15 @@ The Handle Classes
:type: :class:`ChainType`.
+ .. attribute:: hash_code
+
+ A unique identifier for this chain. Note that a deep copy of an entity (see
+ :meth:`EntityHandle.Copy`) will have chains with differing identifiers.
+ Shallow copies of the entity preserve the identifier.
+ Also available as :meth:`GetHashCode`.
+
+ :type: int
+
.. attribute:: valid
Validity of handle.
@@ -558,6 +567,12 @@ The Handle Classes
See :attr:`description`
+ .. method:: GetHashCode()
+
+ See :attr:`hash_code`
+
+ :rtype: int
+
.. method:: IsValid()
See :attr:`valid`
@@ -784,6 +799,15 @@ The Handle Classes
:type: :class:`~ost.geom.Vec3`
+ .. attribute:: hash_code
+
+ A unique identifier for this residue. Note that a deep copy of an entity (see
+ :meth:`EntityHandle.Copy`) will have residues with differing identifiers.
+ Shallow copies of the entity preserve the identifier. Also available as
+ :meth:`GetHashCode`.
+
+ :type: int
+
.. attribute:: valid
Validity of handle.
@@ -959,6 +983,12 @@ The Handle Classes
See :attr:`central_normal`
+ .. method:: GetHashCode()
+
+ See :attr:`hash_code`
+
+ :rtype: int
+
.. method:: IsValid()
See :attr:`valid`
@@ -1099,7 +1129,8 @@ The Handle Classes
:meth:`EntityHandle.Copy`) will have atoms with differing identifiers.
Shallow copies of the entity preserve the identifier. Atom views on a handle
have different identifiers, but the atom view handles (see
- :attr:`AtomView.handle`) have the same identifier.
+ :attr:`AtomView.handle`) have the same identifier. Also available
+ as :meth:`GetHashCode`.
:type: int
@@ -1331,7 +1362,7 @@ The View Classes
.. class:: EntityView
An entity view represents a structural subset of an :class:`EntityHandle`. For
- an introduction ,see :doc:`../../intro-01`.
+ an introduction, see :doc:`../../intro-01`.
.. attribute:: handle
@@ -1703,6 +1734,14 @@ The View Classes
:type: :class:`ChainHandle`
+ .. attribute:: hash_code
+
+ A unique identifier for this chain view. Note, that this is not the same as
+ for the chain handle (see :attr:`ChainHandle.hash_code`).
+ Also available as :meth:`GetHashCode`.
+
+ :type: int
+
.. attribute:: name
The chain name. The name uniquely identifies the chain in the entity. In
@@ -1863,6 +1902,10 @@ The View Classes
See :attr:`handle`
+ .. method:: GetHashCode()
+
+ See :attr:`hash_code`
+
.. method:: GetMass()
See :attr:`mass`
@@ -1927,6 +1970,14 @@ The View Classes
:type: :class:`ResidueHandle`
+ .. attribute:: hash_code
+
+ A unique identifier for this residue view. Note, that this is not the same as
+ for the residue handle (see :attr:`ResidueHandle.hash_code`).
+ Also available as :meth:`GetHashCode`.
+
+ :type: int
+
.. attribute:: name
qualified_name
number
@@ -1994,6 +2045,10 @@ The View Classes
See :attr:`handle`
+ .. method:: GetHashCode()
+
+ See :attr:`hash_code`
+
.. method:: GetName
GetQualifiedName
GetNumber
@@ -2093,6 +2148,7 @@ The View Classes
A unique identifier for this atom view. Note, that this is not the same as
for the atom handle (see :attr:`AtomHandle.hash_code`).
+ Also available as :meth:`GetHashCode`.
:type: int
diff --git a/modules/mol/base/pymod/export_chain.cc b/modules/mol/base/pymod/export_chain.cc
index b5ce6d91c..d358cc198 100644
--- a/modules/mol/base/pymod/export_chain.cc
+++ b/modules/mol/base/pymod/export_chain.cc
@@ -116,6 +116,9 @@ void export_Chain()
.def("GetGeometricEnd", geom_end<ChainHandle>)
.def(self==self)
.def(self!=self)
+ .def("__hash__", &ChainHandle::GetHashCode)
+ .def("GetHashCode", &ChainHandle::GetHashCode)
+ .add_property("hash_code", &ChainHandle::GetHashCode)
;
class_<ChainHandleList>("ChainHandleList", no_init)
diff --git a/modules/mol/base/pymod/export_chain_view.cc b/modules/mol/base/pymod/export_chain_view.cc
index 3587cc572..7c68ee8f1 100644
--- a/modules/mol/base/pymod/export_chain_view.cc
+++ b/modules/mol/base/pymod/export_chain_view.cc
@@ -113,6 +113,9 @@ void export_ChainView()
.add_property("bounds", &ChainView::GetBounds)
.def(self==self)
.def(self!=self)
+ .def("__hash__", &ChainView::GetHashCode)
+ .def("GetHashCode", &ChainView::GetHashCode)
+ .add_property("hash_code", &ChainView::GetHashCode)
;
diff --git a/modules/mol/base/pymod/export_residue_view.cc b/modules/mol/base/pymod/export_residue_view.cc
index 9d14320de..62b8ac413 100644
--- a/modules/mol/base/pymod/export_residue_view.cc
+++ b/modules/mol/base/pymod/export_residue_view.cc
@@ -94,7 +94,10 @@ void export_ResidueView()
.def("GetGeometricStart", geom_start<ResidueView>)
.def("GetGeometricEnd", geom_end<ResidueView>)
.def("GetBounds", &ResidueView::GetBounds)
- .add_property("bounds", &ResidueView::GetBounds)
+ .add_property("bounds", &ResidueView::GetBounds)
+ .def("__hash__", &ResidueView::GetHashCode)
+ .def("GetHashCode", &ResidueView::GetHashCode)
+ .add_property("hash_code", &ResidueView::GetHashCode)
;
diff --git a/modules/mol/base/src/atom_handle.cc b/modules/mol/base/src/atom_handle.cc
index 25eba43f8..ae3bcf920 100644
--- a/modules/mol/base/src/atom_handle.cc
+++ b/modules/mol/base/src/atom_handle.cc
@@ -116,10 +116,10 @@ AtomHandle AtomHandle::GetHandle() const
return *this;
}
-long AtomHandle::GetHashCode() const
+unsigned long AtomHandle::GetHashCode() const
{
this->CheckValidity();
- return reinterpret_cast<long>(Impl().get());
+ return reinterpret_cast<unsigned long>(Impl().get());
}
}} // ns
diff --git a/modules/mol/base/src/atom_handle.hh b/modules/mol/base/src/atom_handle.hh
index 1dbdf09d5..fca1922e9 100644
--- a/modules/mol/base/src/atom_handle.hh
+++ b/modules/mol/base/src/atom_handle.hh
@@ -87,7 +87,7 @@ public:
///
/// Get hash code that uniquely identifies every atom. The hash code is
/// identical for all atom views pointing to a given atom.
- long GetHashCode() const;
+ unsigned long GetHashCode() const;
bool operator==(const AtomHandle& ref) const;
bool operator!=(const AtomHandle& ref) const;
diff --git a/modules/mol/base/src/atom_view.cc b/modules/mol/base/src/atom_view.cc
index 2bd424c8e..5fcb7010a 100644
--- a/modules/mol/base/src/atom_view.cc
+++ b/modules/mol/base/src/atom_view.cc
@@ -161,10 +161,10 @@ void AtomView::RemoveBondInternal(const BondHandle& bond)
}
}
-long AtomView::GetHashCode() const
+unsigned long AtomView::GetHashCode() const
{
this->CheckValidity();
- return reinterpret_cast<long>(data_.get());
+ return reinterpret_cast<unsigned long>(data_.get());
}
}} // ns
diff --git a/modules/mol/base/src/atom_view.hh b/modules/mol/base/src/atom_view.hh
index a853669c1..217245393 100644
--- a/modules/mol/base/src/atom_view.hh
+++ b/modules/mol/base/src/atom_view.hh
@@ -87,7 +87,7 @@ public:
///
/// The unique id is the same for all AtomViews pointing to the same atom
/// view data.
- long GetHashCode() const;
+ unsigned long GetHashCode() const;
bool operator==(const AtomView& rhs) const;
bool operator!=(const AtomView& rhs) const;
protected:
diff --git a/modules/mol/base/src/bond_handle.cc b/modules/mol/base/src/bond_handle.cc
index 9dd04ca12..f9c95d688 100644
--- a/modules/mol/base/src/bond_handle.cc
+++ b/modules/mol/base/src/bond_handle.cc
@@ -122,6 +122,7 @@ void BondHandle::Apply(EntityViewVisitor& v)
unsigned long BondHandle::GetHashCode() const
{
+ this->CheckValidity();
return reinterpret_cast<unsigned long>(impl_.get());
}
diff --git a/modules/mol/base/src/chain_handle.cc b/modules/mol/base/src/chain_handle.cc
index 5ecc5c1fb..bef637465 100644
--- a/modules/mol/base/src/chain_handle.cc
+++ b/modules/mol/base/src/chain_handle.cc
@@ -205,6 +205,7 @@ void ChainHandle::SetInSequence(const int index)
unsigned long ChainHandle::GetHashCode() const
{
+ this->CheckValidity();
return reinterpret_cast<unsigned long>(Impl().get());
}
diff --git a/modules/mol/base/src/chain_view.cc b/modules/mol/base/src/chain_view.cc
index e0c5b5dce..c8498b1d3 100644
--- a/modules/mol/base/src/chain_view.cc
+++ b/modules/mol/base/src/chain_view.cc
@@ -477,5 +477,11 @@ bool ChainView::HasAtoms() const {
return false;
}
+unsigned long ChainView::GetHashCode() const
+{
+ this->CheckValidity();
+ return reinterpret_cast<unsigned long>(data_.get());
+}
+
}} // ns
diff --git a/modules/mol/base/src/chain_view.hh b/modules/mol/base/src/chain_view.hh
index c107261d3..6f2c1a337 100644
--- a/modules/mol/base/src/chain_view.hh
+++ b/modules/mol/base/src/chain_view.hh
@@ -196,6 +196,12 @@ public:
/// \brief return view based on query String.
/// \sa Query
EntityView Select(const String& query_string, QueryFlags flags=0) const;
+
+ /// \brief get unique id
+ ///
+ /// The unique id is the same for all ChainViews pointing to the same chain
+ /// view data.
+ unsigned long GetHashCode() const;
bool operator==(const ChainView& rhs) const;
bool operator!=(const ChainView& rhs) const;
diff --git a/modules/mol/base/src/residue_handle.cc b/modules/mol/base/src/residue_handle.cc
index a355f7fcf..ee0a8d0c1 100644
--- a/modules/mol/base/src/residue_handle.cc
+++ b/modules/mol/base/src/residue_handle.cc
@@ -210,6 +210,7 @@ double ResidueHandle::GetMass() const
unsigned long ResidueHandle::GetHashCode() const
{
+ this->CheckValidity();
return reinterpret_cast<unsigned long>(Impl().get());
}
diff --git a/modules/mol/base/src/residue_view.cc b/modules/mol/base/src/residue_view.cc
index db5503407..5948a38a4 100644
--- a/modules/mol/base/src/residue_view.cc
+++ b/modules/mol/base/src/residue_view.cc
@@ -285,4 +285,10 @@ bool ResidueView::HasAtoms() const {
return data_->atoms.size()>0;
}
+unsigned long ResidueView::GetHashCode() const
+{
+ this->CheckValidity();
+ return reinterpret_cast<unsigned long>(data_.get());
+}
+
}} //ns
diff --git a/modules/mol/base/src/residue_view.hh b/modules/mol/base/src/residue_view.hh
index 518de3f1a..8268143fa 100644
--- a/modules/mol/base/src/residue_view.hh
+++ b/modules/mol/base/src/residue_view.hh
@@ -178,6 +178,12 @@ public:
/// \brief return view based on query String.
/// \sa Query
EntityView Select(const String& query_string, QueryFlags flags=0) const;
+
+ /// \brief get unique id
+ ///
+ /// The unique id is the same for all ResidueViews pointing to the same residue
+ /// view data.
+ unsigned long GetHashCode() const;
bool operator==(const ResidueView& rhs) const;
diff --git a/modules/mol/base/tests/test_residue.cc b/modules/mol/base/tests/test_residue.cc
index 17ef6ee59..1da38c3e3 100644
--- a/modules/mol/base/tests/test_residue.cc
+++ b/modules/mol/base/tests/test_residue.cc
@@ -88,6 +88,35 @@ BOOST_AUTO_TEST_CASE(throw_invalid_res_view)
BOOST_CHECK_NO_THROW(CheckHandleValidity(res));
}
+// Test that .GetHashCode() throws on invalid residue handle
+BOOST_AUTO_TEST_CASE(throw_hash_code_invalid_res_handle)
+{
+ ChainHandle chain;
+ EntityHandle ent=CreateEntity();
+ XCSEditor edi=ent.EditXCS();
+ chain=edi.InsertChain("A");
+ ResidueHandle res=chain.FindResidue(ResNum(1));
+ BOOST_CHECK_THROW(res.GetHashCode(), InvalidHandle);
+ edi.AppendResidue(chain, "GLY");
+ res=chain.FindResidue(ResNum(1));
+ BOOST_CHECK_NO_THROW(res.GetHashCode());
+}
+
+// Test that .GetHashCode() throws on invalid residue views
+BOOST_AUTO_TEST_CASE(throw_hash_code_invalid_res_view)
+{
+ ChainHandle chain;
+ EntityHandle ent=CreateEntity();
+ XCSEditor edi=ent.EditXCS();
+ chain=edi.InsertChain("A");
+ ResidueView res;
+ BOOST_CHECK_THROW(res.GetHashCode(), InvalidHandle);
+ edi.AppendResidue(chain, "GLY");
+ EntityView ent_view=ent.CreateFullView();
+ res=ent_view.FindChain("A").FindResidue(1);
+ BOOST_CHECK_NO_THROW(res.GetHashCode());
+}
+
BOOST_AUTO_TEST_CASE(test_res_index)
{
EntityHandle eh=CreateEntity();
--
GitLab