Skip to content

test for compute_cve() #78

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 4 commits into from
Jun 20, 2024
Merged

test for compute_cve() #78

merged 4 commits into from
Jun 20, 2024

Conversation

yucongalicechen
Copy link
Collaborator

  • closes test for compute_cve() #20
  • Initial commit
  • I have to add the line n_points_on_diameter=N_POINTS_ON_DIAMETER to make monkeypatch work in the test. It doesn't affect the current functionality but is probably not the desired behavior.. Any suggestions for this?

@sbillinge
Copy link
Contributor

sbillinge commented Jun 14, 2024 via email

@sbillinge
Copy link
Contributor

this is failing CI. It can't find mocker for some reason. I don't immediately see the problem, but please take a look.

@yucongalicechen
Copy link
Collaborator Author

Oh okay, I will look into this.
I'm also thinking if we want to update package info in this function. It might be easier to maintain/check if we load the package info into "input_pattern" DO along with other things in args. But would it make more sense to update the package here?

@sbillinge
Copy link
Contributor

Oh okay, I will look into this. I'm also thinking if we want to update package info in this function. It might be easier to maintain/check if we load the package info into "input_pattern" DO along with other things in args. But would it make more sense to update the package here?

I'm not really sure. I think in general we may want to update package_info whenever a package is used. The way we implemented it was that it was a set, so each package would only appear once, regardless how often it is used. From this point of view, we may want to just update labpdfproc once near the beginning of when it is used. If it doesn't already, we may want diffpy_utils to update whenever a DO is instantiated, but that would happen in diffpy.utils not here.

tl;dr, I don't think we need it here, but I do think we need it in labpdfprocapp somewhere.

@yucongalicechen
Copy link
Collaborator Author

Got it. So this would be similar to loading user info into args. I'll talk to Steven to see if he could work on that.

@sbillinge sbillinge merged commit 069fb72 into diffpy:main Jun 20, 2024
3 checks passed
@sbillinge
Copy link
Contributor

nice!

@yucongalicechen yucongalicechen deleted the cve2 branch June 20, 2024 13:46
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.

test for compute_cve()
2 participants