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

Deploy as dls dodal #164

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Deploy as dls dodal #164

merged 3 commits into from
Sep 12, 2023

Conversation

DominicOram
Copy link
Contributor

Fixes #145

This will mean that dodal is installed by pip install dls-dodal but used with import dodal. I think this is fine, various other python packages have a different distribution name .vs import name but interested if others disagree.

To test this pip install https://test.pypi.org/project/dls-dodal/ (this is from https://github.com/DominicOram/dodal/commits/0.0.8, which is this branch but pointed at test pypi and a fixed ophyd version)

To actually release we will need to:

Copy link
Collaborator

@graeme-winter graeme-winter left a comment

Choose a reason for hiding this comment

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

Seems like a sensible action - FWIW this is a pretty common pattern as far as I can tell

Actual change set IRL is tiny - do any docs need updating to go alongside this?

Copy link
Contributor

@Tom-Willemsen Tom-Willemsen left a comment

Choose a reason for hiding this comment

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

This will mean that dodal is installed by pip install dls-dodal but used with import dodal. I think this is fine

I agree, this seems fine, lots of packages do this and there is minimal risk of confusion/conflicts here.

Convince ophyd to make a release

I guess this might be complicated by the recent split to ophyd / ophyd-async - I think we probably need both to release as it would be confusing to have two copies of ophyd V2 in our dependencies (and we'll likely need to adjust a bunch of our imports etc anyway)?

@DominicOram
Copy link
Contributor Author

Seems like a sensible action - FWIW this is a pretty common pattern as far as I can tell

Actual change set IRL is tiny - do any docs need updating to go alongside this?

Docs were changed in #72. So we've actually been in a state where the docs are wrong for the last couple of months.

I guess this might be complicated by the recent split to ophyd / ophyd-async - I think we probably need both to release as it would be confusing to have two copies of ophyd V2 in our dependencies (and we'll likely need to adjust a bunch of our imports etc anyway)?

Yes, you're right that now we're using v2 we'll need them to release both. I think the removal of v2 is going to happen at the end of this week but I can discuss with them next week

@DominicOram
Copy link
Contributor Author

Given this affects blueapi a bit too I will wait on @callumforrester before merging

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

Happy to go ahead

@DominicOram DominicOram merged commit 7058d11 into main Sep 12, 2023
14 checks passed
@DominicOram DominicOram deleted the 145_deploy_as_dls_dodal branch September 12, 2023 08:15
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.

Rename to allow pypi hosting
4 participants