Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gettable id property to DiffractionObject #216

Merged
merged 9 commits into from
Dec 13, 2024
23 changes: 23 additions & 0 deletions news/uuid.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* Gettable `id` property to `DiffractionObject`

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
10 changes: 10 additions & 0 deletions src/diffpy/utils/diffraction_objects.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import datetime
import uuid
import warnings
from copy import deepcopy

Expand Down Expand Up @@ -53,6 +54,7 @@ def __init__(
if yarray is None:
yarray = np.empty(0)

self._id = uuid.uuid4()
self.input_data(xarray, yarray, xtype)

def __eq__(self, other):
Expand Down Expand Up @@ -205,6 +207,14 @@ def input_xtype(self):
def input_xtype(self, _):
raise AttributeError(_setter_wmsg("input_xtype"))

@property
def id(self):
return self._id

@id.setter
def id(self, _):
raise AttributeError(_setter_wmsg("id"))

def set_angles_from_list(self, angles_list):
self.angles = angles_list
self.n_steps = len(angles_list) - 1.0
Expand Down
31 changes: 28 additions & 3 deletions tests/test_diffraction_objects.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import re
import uuid
from pathlib import Path
from uuid import UUID

import numpy as np
import pytest
Expand Down Expand Up @@ -335,8 +337,8 @@ def test_dump(tmp_path, mocker):

@pytest.mark.parametrize("inputs, expected", tc_params)
def test_constructor(inputs, expected):
actual_do = DiffractionObject(**inputs)
diff = DeepDiff(actual_do.__dict__, expected, ignore_order=True, significant_digits=13)
actual = DiffractionObject(**inputs).__dict__
diff = DeepDiff(actual, expected, ignore_order=True, significant_digits=13, exclude_paths="root['_id']")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for testing constructor, _id is ignored since it's hard to dynamically compare its uuid value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that ok? not sure if there is a better way for this.

Copy link
Contributor

@sbillinge sbillinge Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the past I have mocked that attribute. This here seems less work. If we have tests that test the id setting and getting, we can safely ignore it in other tests that are testing other things, such as this one, think

Copy link
Contributor Author

@bobleesj bobleesj Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted - yes is setter/getter test funcs provided below.

assert diff == {}


Expand Down Expand Up @@ -369,6 +371,29 @@ def test_all_array_setter():
actual_do.all_arrays = np.empty((4, 4))


def test_id_getter():
do = DiffractionObject()
assert hasattr(do, "id")
assert isinstance(do.id, UUID)
assert len(str(do.id)) == 36


def test_id_getter_with_mock(mocker):
mocker.patch.object(DiffractionObject, "id", new_callable=lambda: UUID("d67b19c6-3016-439f-81f7-cf20a04bee87"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since id is used with @property, the following doesn't apply:

mocker.patch.object(DiffractionObject, 'id', return_value="d67b19c6-3016-439f-81f7-cf20a04bee87")

Hence we use the new_callable parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, nicely done. I didn't know about that.

do = DiffractionObject()
assert do.id == UUID("d67b19c6-3016-439f-81f7-cf20a04bee87")


def test_id_setter_error():
do = DiffractionObject()

with pytest.raises(
AttributeError,
match="Direct modification of attribute 'id' is not allowed. Please use 'input_data' to modify 'id'.",
):
do.id = uuid.uuid4()


def test_xarray_yarray_length_mismatch():
with pytest.raises(
ValueError,
Expand All @@ -384,7 +409,7 @@ def test_input_xtype_getter():
assert do.input_xtype == "tth"


def test_input_xtype_setter():
def test_input_xtype_setter_error():
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just enhancing the function name for clarify

do = DiffractionObject(xtype="tth")

# Attempt to directly modify the property
Expand Down
Loading