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

Hidden xray-vision dependency is still not dealt with anywhere #1636

Open
flowln opened this issue Dec 7, 2023 · 5 comments · May be fixed by #1726
Open

Hidden xray-vision dependency is still not dealt with anywhere #1636

flowln opened this issue Dec 7, 2023 · 5 comments · May be fixed by #1726
Assignees
Labels
good first issue hackathon Good issues for the upcoming bluesky hackathon

Comments

@flowln
Copy link

flowln commented Dec 7, 2023

There's still a xray-vision hidden dependency in the LiveImage callback, mentioned in previous issues (#371 #823). This dependency is not mentioned anywhere in the README, the docs, or the requirements, so you only encounter it if you're reading the code, or if you happily stumble upon a nice ImportError on your bluesky journey 🙂

Expected Behavior

No error should be raised on normal operation, if you install a package directly from where it is made available oficially.

Possible Solution

  1. Vendor-in the relevant code from xray-vision into Bluesky.
  2. Add the xray-vision dependency to the project requirements (possibly optional, like the matplotlib dependency, though it should not make the callback accessible when it is not installed).
@callumforrester callumforrester added hackathon Good issues for the upcoming bluesky hackathon good first issue labels Apr 8, 2024
@callumforrester
Copy link
Contributor

Reviewers: @tacaswell (who thinks vendoring in is the better path)

@stan-dot
Copy link
Contributor

here is the relevant file to vendor-in. To make it work likely the whole package will need to be imported

@tacaswell
Copy link
Contributor

The abstract layers were a bit of architecture astronaut exercise. We only ever ended up with one actual implementation so in the same move we should also squash down what ever it is we are inheriting.

@stan-dot
Copy link
Contributor

The mpl_package supports three kinds of display: CrossSection, Stack1d and Contour. We only use the first one. Should all the others be deleted?
@tacaswell

@tacaswell
Copy link
Contributor

I would phrase it as "only port what we need", but yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue hackathon Good issues for the upcoming bluesky hackathon
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants