Skip to content

Fix: let DiffractionObject.get_array_index to use xtype from its inputs. #356

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 25, 2025
Merged

Fix: let DiffractionObject.get_array_index to use xtype from its inputs. #356

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e43660
test: update the test code for `DiffractionObject.get_array_index`
ycexiao Jul 25, 2025
bedf468
fix: let `DiffractionObject.get_array_index` to use an optional input…
ycexiao Jul 25, 2025
eaeafe5
chore: fix pre-commit
ycexiao Jul 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/fix-get-array-index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* <news item>

**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* Let ``DiffractionObject.get_array_index`` to use the ``xtype`` from its inputs.

**Security:**

* <news item>
10 changes: 5 additions & 5 deletions tests/test_diffraction_objects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ def test_scale_to_bad(org_do_args, target_do_args, scale_inputs):
},
0,
),
( # C2: Target value lies in the array, expect the (first) closest
# index
# C2: Target value lies in the array, expect the closest index
( # 1. same xtype
Copy link
Contributor

@sbillinge sbillinge Jul 25, 2025

Choose a reason for hiding this comment

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

what does "same xtype" mean? same as what?

Copy link
Contributor Author

@ycexiao ycexiao Jul 25, 2025

Choose a reason for hiding this comment

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

It means that the xtype of the original DiffractionObject and the xtype of the data from which the get_array_index gets the array index are the same.

I will modify the comment.

Copy link
Contributor

@sbillinge sbillinge Jul 25, 2025

Choose a reason for hiding this comment

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

This may not need to be changed depending on the outcome of the discussion above. I am thinking that we may want to make both variables required. Do you know of a particular UC that requires the other? Do we now anywhere where this is being used actually?

Copy link
Contributor Author

@ycexiao ycexiao Jul 25, 2025

Choose a reason for hiding this comment

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

Do you know of a particular UC that requires the other?

No. I think what the function does is to find the index. xtype is not actually needed.

And if users want to locate a specific q or tth that is different from the _input_xtype, they can use diff_obj.on_xtype("<xtype>"). Also, this might not be a common need.

Do we now anywhere where this is being used actually?

At least in diffpy.utils, it is not referenced. It is just a helper function we provide to users.

{
"wavelength": 4 * np.pi,
"xarray": np.array([30, 60]),
Expand All @@ -390,7 +390,7 @@ def test_scale_to_bad(org_do_args, target_do_args, scale_inputs):
},
0,
),
(
( # 2. different xtype
Copy link
Contributor Author

@ycexiao ycexiao Jul 25, 2025

Choose a reason for hiding this comment

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

There was a test for the function. I modified it so that it can capture the bug now.

{
"wavelength": 4 * np.pi,
"xarray": np.array([30, 60]),
Expand All @@ -399,9 +399,9 @@ def test_scale_to_bad(org_do_args, target_do_args, scale_inputs):
},
{
"xtype": "q",
"value": 0.25,
"value": 0.5,
},
0,
1,
),
# C3: Target value out of the range, expect the closest index
( # 1. Test with xtype of "q"
Expand Down
Loading