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

restore functionality with array-like tth input #440

Merged
merged 1 commit into from
May 17, 2022

Conversation

joelvbernier
Copy link
Member

@joelvbernier joelvbernier commented May 11, 2022

A recent merge broke the ability to use instrument.HEDMInstrument.extract_line_positions() with array-like input for the plane_data arg. This PR restores that functionality. Some default values are also updated, noting that the 2theta tolerance is now in % difference, not absolute deg.

closes #441

@joelvbernier joelvbernier added the invalid This doesn't seem right label May 11, 2022
@joelvbernier joelvbernier requested a review from psavery May 11, 2022 00:31
@joelvbernier joelvbernier self-assigned this May 11, 2022
@pep8speaks
Copy link

pep8speaks commented May 12, 2022

Hello @joelvbernier! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-05-13 08:26:57 UTC

@joelvbernier
Copy link
Member Author

Checks out on Linux; seems to fail for mp_context=spawn however.

@joelvbernier
Copy link
Member Author

@psavery -- confirmed that the attached problem fails when I try to run under the spawn context on Mac and WIndows. Runs fine under Linux/fork.
DCS_powder_cal_example.zip

@psavery
Copy link
Collaborator

psavery commented May 16, 2022

I think you just need to change the last line in your script to something like this:

if __name__ == '__main__':
    import multiprocessing
    multiprocessing.freeze_support()
    pwdr_lines = instr.extract_line_positions(**kwargs)

See #441 for more details.

Copy link
Collaborator

@psavery psavery left a comment

Choose a reason for hiding this comment

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

LGTM. It seems that the spawn multiprocessing issue you are encountering is to be fixed in the script, rather than in this PR.

@joelvbernier joelvbernier merged commit 66b56d6 into master May 17, 2022
@psavery psavery deleted the powder-line-extraction branch May 17, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broken functionality in instrument.HEDMInstrument.extract_line_positions()
3 participants