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

Install pango_aliasor #146

Merged
merged 1 commit into from
May 17, 2023
Merged

Install pango_aliasor #146

merged 1 commit into from
May 17, 2023

Conversation

corneliusroemer
Copy link
Member

@corneliusroemer corneliusroemer commented May 4, 2023

Package maintained by myself to simplify aliasing and dealising of pango lineages

Package is pure Python without dependencies and minimal size

Source code: https://github.com/corneliusroemer/pango_aliasor

See further discussion: https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1683137801980639

Blocking for nextstrain/forecasts-ncov#39

TODO:

Package maintained by myself to simplify aliasing and dealising of pango lineages

Package is pure Python without dependencies and minimal size

Source code: https://github.com/corneliusroemer/pango_aliasor

See further discussion: https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1683137801980639

Blocking for nextstrain/forecasts-ncov#39
@corneliusroemer corneliusroemer requested a review from a team May 8, 2023 10:51
@corneliusroemer
Copy link
Member Author

Anything holding this up? It's blocking for nextstrain/forecasts-ncov#39

@trvrb
Copy link
Member

trvrb commented May 8, 2023

Looks good to me, though I haven't done any updating to docker-base in a while and so I don't know if there's anything we need to do to trigger image rebuild or if this will work automatically.

@tsibley or @victorlin: Okay to merge this?

Copy link
Member

@victorlin victorlin left a comment

Choose a reason for hiding this comment

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

Looks good to me, though we had bigger picture discussions in Slack, i.e. maybe forecasts-ncov should maintain its own Docker image that extends nextstrain/base with evofr and pango_aliasor instead of relying on them here.

@tsibley
Copy link
Member

tsibley commented May 8, 2023

This seems fine modulo what @victorlin said, but @corneliusroemer please do follow up sooner than later with the "Parallel addition to nextstrain/conda-base" todo.

@tsibley
Copy link
Member

tsibley commented May 9, 2023

@trvrb

I don't know if there's anything we need to do to trigger image rebuild or if this will work automatically.

It's fully automatic. …but we're currently experiencing a CI breakage due to a change from this morning. A proposed fix is currently being tested. I plan to resolve it one way or another tomorrow morning.

@corneliusroemer
Copy link
Member Author

If we were to make more workflow specific images on top of docker-base, we could remove pango_aliasor again. All repos that were to use that package would then just have to make their own images - but that number isn't going to be much larger than 1 anyways. So I don't think that's blocking.

corneliusroemer added a commit to nextstrain/conda-base that referenced this pull request May 9, 2023
@joverlee521
Copy link
Contributor

Merging now that our CI and cache issues have been fixed.

@joverlee521 joverlee521 merged commit 1b0d199 into master May 17, 2023
@joverlee521 joverlee521 deleted the add-pango-aliasor branch May 17, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

5 participants