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

Corrected SPEC header formation from bluesky start doc #17

Merged
merged 6 commits into from
Mar 31, 2022

Conversation

whs92
Copy link
Member

@whs92 whs92 commented Apr 12, 2021

the spec header was incorrectly being written as 'seq_num' rather than being populated with the plan name and the plan arguments even when the plan_name was in _BLUESKY_PLAN_NAMES. I corrected this by checking the scan_command was in _SPEC_SCAN_NAMES rather than _BLUESKY_PLAN_NAMES. I then changed the command_args calculation to assume that args was a list and not a dict since this is what is implemented for the scan and rel_scan plans. Tested by running scans and opening them in pyMca

…n being populated with the plan name and the plan arguments. I corrected this by checking the scan_command was in _SPEC_SCAN_NAMES rather than _BLUESKY_PLAN_NAMES. I then changed the command_args calculation to assume that 'args' was a list and not a dict since this is what is implemented for the scan and rel_scan plans. Tested by running scans and opening them in pyMca
@whs92
Copy link
Member Author

whs92 commented Apr 12, 2021

Fails test because test data also contains error with no motor specified and instead just "seq_num". JSON files also seem to contain different data to the specfiles they are comparing against and test fails on master branch too.

@mrakitin mrakitin requested review from gwbischof and jklynch April 12, 2021 16:50
@mrakitin
Copy link
Member

Thanks for the contribution/fix, @whs92. I haven't participated in this suitcase's development, however, @gwbischof and @jklynch were involved. I've requested their review.

For the failing test, I think it would make sense to correct the .spec files to make them more compliant with the SPEC format.

@jklynch
Copy link
Contributor

jklynch commented Apr 12, 2021

I have not participated in developing the "working" code for this project. My involvement has been purely peripheral, and I don't know anything about spec. I can't provide a helpful review.

@mrakitin mrakitin requested review from danielballan and removed request for jklynch April 12, 2021 19:18
@danielballan
Copy link
Member

This is challenging to test well because there is no SPEC specification --- it's just an undocumented convention --- and different implementations have conflicting expectations. This one is intended to work with the implementation in pyMCA, which I believe is fortunately also the one that @whs92 is targeting.

I'll take a look....

@danielballan
Copy link
Member

Fixes proposed in hz-b#1.

@danielballan danielballan merged commit 793f6e5 into bluesky:master Mar 31, 2022
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.

4 participants