-
Notifications
You must be signed in to change notification settings - Fork 20
improve comments for test_scale_to
#280
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
Changes from 2 commits
56e7e83
0a30818
a2a43a0
5fc00b2
d5fd01f
07938c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| **Added:** | ||
|
|
||
| * no news added - make test comments for `test_scale_to` more readable | ||
|
|
||
| **Changed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Deprecated:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Removed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Fixed:** | ||
|
|
||
| * <news item> | ||
|
|
||
| **Security:** | ||
|
|
||
| * <news item> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,7 +191,7 @@ def test_init_invalid_xtype(): | |
| "org_do_args, target_do_args, scale_inputs, expected", | ||
| [ | ||
| # Test whether scale_to() scales to the expected values | ||
| ( # C1: Same x-array and y-array with 2.1 offset, expect yarray shifted by 2.1 offset | ||
| ( # C1: Same x-array and y-array with 2.1 offset, expect y-array to shift up by 2.1 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clear |
||
| { | ||
| "xarray": np.array([10, 15, 25, 30, 60, 140]), | ||
| "yarray": np.array([2, 3, 4, 5, 6, 7]), | ||
|
|
@@ -212,7 +212,7 @@ def test_init_invalid_xtype(): | |
| }, | ||
| {"xtype": "tth", "yarray": np.array([4.1, 5.1, 6.1, 7.1, 8.1, 9.1])}, | ||
| ), | ||
| ( # C2: Same length x-arrays with exact x-value match | ||
| ( # C2: Same x-arrays, x-value matches at tth=60, expect y-array to divide by 10 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If i understand correctly, we compare the yarray_1 of 60 against yarray_2 of 6 (located at xarray of 60) and we expect the y-array to e divided by 10 based on this ratio? Just comment - since both xarray and yarray use 60 for the first input, it's a bit hard to follow
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! It finds the closest index in each x-array and scales based on that. In this case the scaling factor is 60/6=10 and the y-array for the original diffraction object is divided by 10 |
||
| { | ||
| "xarray": np.array([10, 15, 25, 30, 60, 140]), | ||
| "yarray": np.array([10, 20, 25, 30, 60, 100]), | ||
|
|
@@ -233,7 +233,10 @@ def test_init_invalid_xtype(): | |
| }, | ||
| {"xtype": "tth", "yarray": np.array([1, 2, 2.5, 3, 6, 10])}, | ||
| ), | ||
| ( # C3: Same length x-arrays with approximate x-value match | ||
| ( # C3: Different x-arrays with same length, | ||
| # x-value has closest match at q=0.12 (y=10) and q=0.14 (y=1) | ||
| # for original and target diffraction objects, | ||
| # expect y-array to divide by 10 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok - yarray of 10 and 1 are compared since they are the nearest values at q=0.1. Since the second is 1/10 of the original, we expect y value to be 1/10 |
||
| { | ||
| "xarray": np.array([0.12, 0.24, 0.31, 0.4]), | ||
| "yarray": np.array([10, 20, 40, 60]), | ||
|
|
@@ -254,7 +257,10 @@ def test_init_invalid_xtype(): | |
| }, | ||
| {"xtype": "q", "yarray": np.array([1, 2, 4, 6])}, | ||
| ), | ||
| ( # C4: Different x-array lengths with approximate x-value match | ||
| ( # C4: Different x-array lengths | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay - clear, I understand the function now. yeah for C2, it was hard for me to understand what the function does by reading the test alone since numbers were similiar but other cases are clear.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! I'll edit C2 |
||
| # x-value has closest matches at tth=61 (y=50) and tth=62 (y=5), | ||
| # for original and target diffraction objects, | ||
| # expect y-array to divide by 10 | ||
| { | ||
| "xarray": np.array([10, 25, 30.1, 40.2, 61, 120, 140]), | ||
| "yarray": np.array([10, 20, 30, 40, 50, 60, 100]), | ||
|
|
@@ -273,7 +279,6 @@ def test_init_invalid_xtype(): | |
| "d": None, | ||
| "offset": 0, | ||
| }, | ||
| # C5: Scaling factor is calculated at index = 4 (tth=61) for self and index = 5 for target (tth=62) | ||
| {"xtype": "tth", "yarray": np.array([1, 2, 3, 4, 5, 6, 10])}, | ||
| ), | ||
| ], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scales_to() scales tofeels a bit circulating?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops yes forgot to change this - thanks!