Skip to content

add private function for initating DO #81

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

Merged
merged 3 commits into from
Jun 21, 2024
Merged

Conversation

yucongalicechen
Copy link
Collaborator

addressing #80

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

it still seems a bit hard to read. Also the numbers in the yarray are fairly irrational

@@ -56,27 +56,29 @@ def test_set_muls_at_angle(inputs, expected):
assert actual_muls_sorted == pytest.approx(expected_muls_sorted, rel=1e-4, abs=1e-6)


def test_compute_cve(mocker):
def _instantiate_test_do(mocker, yarray, name="test", scat_quantity="x-ray"):
Copy link
Contributor

Choose a reason for hiding this comment

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

if you pass everything in we don't gain much. Are there cases where you want to vary name="test", scat_quantity="x-ray" etc?

I would make yarray a required arg too to make the code more readable.

Also, is it necessary to pass in mocker? It may be, but if not I don't think it adds much syntatically.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I couldn't find prettier numbers for the yarrays.. they all seem a bit messy.
The name and scat_quantity are for the absorption correction DOs.
I can remove the mocker, it's not needed there.

@sbillinge
Copy link
Contributor

sbillinge commented Jun 20, 2024 via email

@yucongalicechen
Copy link
Collaborator Author

I added tests for apply_corr here too. It seems that I can write another private function for the setup to simplify (mocker, input pattern, etc.)..?

@sbillinge sbillinge merged commit 8a9627e into diffpy:main Jun 21, 2024
5 checks passed
This was referenced Jun 21, 2024
@yucongalicechen yucongalicechen deleted the initdo branch June 21, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants