Skip to content

Commit

Permalink
Attribute validation on assignment
Browse files Browse the repository at this point in the history
  • Loading branch information
tovrstra committed Aug 29, 2020
1 parent 0991c46 commit cda1f95
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 32 deletions.
45 changes: 40 additions & 5 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,11 @@ Notes on attrs
IOData uses the `attrs`_ library, not to be confused with the `attr`_ library,
for classes representing data loaded from files: ``IOData``, ``MolecularBasis``,
``Shell``, ``MolecularOrbitals`` and ``Cube``. This enables basic attribute
validation, which eliminates potentially silly bugs. The following two tricks
might be convenient with working with these classes:
validation, which eliminates potentially silly bugs.
(See ``iodata/attrutils.py`` and the usage of ``validate_shape`` in all those
classes.)

The following two tricks might be convenient with working with these classes:

- The data can be turned into plain Python data types with the ``attr.asdict``
function. Make sure you add the ``retain_collection_types=True`` option, to
Expand All @@ -347,15 +350,46 @@ might be convenient with working with these classes:
iodata = load_one("example.xyz")
fields = attr.asdict(iodata, retain_collection_types=True)
- A shallow copy with a few modified attributes can be created with the evolve
method, which is a wrapper for ``attr.evolve``:
A similar ``astuple`` function works as you would expect.

- A `shallow copy`_ with a few modified attributes can be created with the
evolve method, which is a wrapper for ``attr.evolve``:

.. code-block:: python
from iodata import load_one
import attr
iodata1 = load_one("example.xyz")
iodata2 = iodata1.evolve(title="another title")
iodata2 = attr.evolve(iodata1, title="another title")
The usage of evolve becomes mandatory when you want to change two or more
attributes whose shape need to be consistent. For example, the following
would fail:

.. code-block:: python
from iodata import IOData
iodata = IOData(atnums=[7, 7], atcoords=[[0, 0, 0], [2, 0, 0]])
# The next line will fail because the size of atnums and atcoords
# becomes inconsistent.
iodata.atnums = [8, 8, 8]
iodata.atcoords = [[0, 0, 0], [2, 0, 1], [4, 0, 0]]
The following code, which has the same intent, does work:

.. code-block:: python
from iodata import IOData
import attr
iodata1 = IOData(atnums=[7, 7], atcoords=[[0, 0, 0], [2, 0, 0]])
iodata2 = attr.evolve(
iodata1,
atnums=[8, 8, 8],
atcoords=[[0, 0, 0], [2, 0, 1], [4, 0, 0]],
)
For brevity, lists (of lists) were used in these examples. These are always
converted to arrays by the constructor or when assigning them to attributes.
.. _Bash: https://en.wikipedia.org/wiki/Bash_(Unix_shell)
Expand All @@ -364,3 +398,4 @@ might be convenient with working with these classes:
.. _PEP 0563: https://www.python.org/dev/peps/pep-0563/
.. _attrs: https://www.attrs.org/en/stable/
.. _attr: https://github.com/denis-ryzhkov/attr
.. _shallow copy: https://docs.python.org/3/library/copy.html?highlight=shallow
6 changes: 4 additions & 2 deletions iodata/basis.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ def angmom_its(angmom: Union[int, List[int]]) -> Union[str, List[str]]:
return ANGMOM_CHARS[angmom]


@attr.s(auto_attribs=True, slots=True)
@attr.s(auto_attribs=True, slots=True,
on_setattr=[attr.setters.validate, attr.setters.convert])
class Shell:
"""A shell in a molecular basis representing (generalized) contractions with the same exponents.
Expand Down Expand Up @@ -141,7 +142,8 @@ def ncon(self) -> int: # noqa: D401
return len(self.angmoms)


@attr.s(auto_attribs=True, slots=True)
@attr.s(auto_attribs=True, slots=True,
on_setattr=[attr.setters.validate, attr.setters.convert])
class MolecularBasis:
"""A complete molecular orbital or density basis set.
Expand Down
3 changes: 2 additions & 1 deletion iodata/iodata.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@


# pylint: disable=too-many-instance-attributes
@attr.s(auto_attribs=True, slots=True)
@attr.s(auto_attribs=True, slots=True,
on_setattr=[attr.setters.validate, attr.setters.convert])
class IOData:
"""A container class for data loaded from (or to be written to) a file.
Expand Down
3 changes: 2 additions & 1 deletion iodata/orbitals.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
__all__ = ['MolecularOrbitals']


@attr.s(auto_attribs=True, slots=True)
@attr.s(auto_attribs=True, slots=True,
on_setattr=[attr.setters.validate, attr.setters.convert])
class MolecularOrbitals:
"""Class of Orthonormal Molecular Orbitals.
Expand Down
26 changes: 6 additions & 20 deletions iodata/test/test_attrutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from ..attrutils import convert_array_to, validate_shape


@attr.s(slots=True)
@attr.s(auto_attribs=True, slots=True, on_setattr=attr.setters.convert)
class FooBar:
"""Just a silly class for testing convert_array_to."""

Expand All @@ -46,9 +46,6 @@ def test_convert_array_to_init():
assert_allclose(fb.spam, spam2)


# converter is not called when assigning!
# See https://github.com/python-attrs/attrs/issues/645
@pytest.mark.skip
def test_convert_array_to_assign():
fb = FooBar(spam=None)
assert fb.spam is None
Expand All @@ -63,22 +60,21 @@ def test_convert_array_to_assign():
assert fb.spam is None


@attr.s(slots=True, frozen=True)
@attr.s(auto_attribs=True, slots=True, on_setattr=attr.setters.validate)
class Spam:
"""Just a silly class for testing validate_shape."""

egg0: np.ndarray = attr.ib(validator=validate_shape(1, None, "negg"))
egg1: np.ndarray = attr.ib(validator=validate_shape("negg", ("egg2", 1)))
egg0: np.ndarray = attr.ib(validator=validate_shape(1, None, None))
egg1: np.ndarray = attr.ib(validator=validate_shape(("egg0", 2), ("egg2", 1)))
egg2: np.ndarray = attr.ib(validator=validate_shape(2, ("egg1", 1)))
egg3: np.ndarray = attr.ib(validator=validate_shape(("leg", 0)))
negg: int = attr.ib(converter=int)
leg: str = attr.ib(validator=validate_shape(("egg3", 0)))


def test_validate_shape_init():
# Construct a Spam instance with valid arguments. This should just work
spam = Spam(
np.zeros((1, 7, 4)), np.zeros((4, 3)), np.zeros((2, 3)), np.zeros(5), 4, "abcde"
np.zeros((1, 7, 4)), np.zeros((4, 3)), np.zeros((2, 3)), np.zeros(5), "abcde"
)
# Double check
attr.validate(spam)
Expand All @@ -89,7 +85,6 @@ def test_validate_shape_init():
np.zeros((4, 3)),
np.zeros((2, 3)),
np.zeros(5),
4,
"abcde",
)
with pytest.raises(TypeError):
Expand All @@ -98,7 +93,6 @@ def test_validate_shape_init():
np.zeros((4, 3)),
np.zeros((2, 3)),
np.zeros(5),
4,
"abcde",
)
with pytest.raises(TypeError):
Expand All @@ -107,7 +101,6 @@ def test_validate_shape_init():
np.zeros((1, 3)),
np.zeros((2, 3)),
np.zeros(5),
4,
"abcde",
)
with pytest.raises(TypeError):
Expand All @@ -116,7 +109,6 @@ def test_validate_shape_init():
np.zeros((4, 9)),
np.zeros((2, 3)),
np.zeros(5),
4,
"abcde",
)
with pytest.raises(TypeError):
Expand All @@ -125,7 +117,6 @@ def test_validate_shape_init():
np.zeros((4, 3)),
np.zeros((2, 3)),
np.zeros(5),
15,
"abcde",
)
with pytest.raises(TypeError):
Expand All @@ -139,13 +130,10 @@ def test_validate_shape_init():
)


# validator is not called when assigning.
# See https://github.com/python-attrs/attrs/issues/645
@pytest.mark.skip
def test_validate_shape_assign():
# Construct a Spam instance with valid arguments. This should just work
spam = Spam(
np.zeros((1, 7, 4)), np.zeros((4, 3)), np.zeros((2, 3)), np.zeros(5), 4, "abcde"
np.zeros((1, 7, 4)), np.zeros((4, 3)), np.zeros((2, 3)), np.zeros(5), "abcde"
)
# Double check
attr.validate(spam)
Expand All @@ -158,8 +146,6 @@ def test_validate_shape_assign():
spam.egg1 = np.zeros((1, 3))
with pytest.raises(TypeError):
spam.egg1 = np.zeros((4, 9))
with pytest.raises(TypeError):
spam.negg = 15
with pytest.raises(TypeError):
spam.leg = "abcd"

Expand Down
3 changes: 2 additions & 1 deletion iodata/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ def back(self, line):
self.lineno -= 1


@attr.s(auto_attribs=True, slots=True)
@attr.s(auto_attribs=True, slots=True,
on_setattr=[attr.setters.validate, attr.setters.convert])
class Cube:
"""The volumetric data from a cube (or similar) file.
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,6 @@ def get_readme():
'Intended Audience :: Science/Research',
],
setup_requires=['numpy>=1.0'],
install_requires=['numpy>=1.0', 'scipy', 'attrs>=19.1.0',
install_requires=['numpy>=1.0', 'scipy', 'attrs>=20.1.0',
'importlib_resources; python_version < "3.7"'],
)
2 changes: 1 addition & 1 deletion tools/conda.recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ requirements:
run:
- python
- scipy
- attrs >=19.1.0
- attrs >=20.1.0
- importlib_resources # [py<37]

test:
Expand Down

0 comments on commit cda1f95

Please sign in to comment.