nextstrain / public

repo to house broad planning issues and other cross-repo concerns; see also https://github.com/nextstrain/private
0 stars 0 forks source link

Replace set_final_strain_name.py #5

Open victorlin opened 3 months ago

victorlin commented 3 months ago

_initially brought up by @joverlee521 in https://github.com/nextstrain/dengue/pull/12#discussion_r1357171352 and resurfaced on Slack_

The script is a not-so-ideal workaround for the problems that come from using a custom metadata ID column. Better solutions:

Update pathogen repos

GitHub search reveals this is used in:

victorlin commented 2 months ago

https://github.com/nextstrain/auspice/pull/1668 has been merged and released. I'm not sure what the next step is for this issue. From @joverlee521 https://github.com/nextstrain/augur/issues/1264#issuecomment-1652472124:

Then we would need to add tip_label as an allowed property for display_defaults in our auspice config schema.

I also think export v2 should verify the tip_label field is an available node_attr or automatically add it as a node_attr if the field is available in the metadata file.

Is https://github.com/nextstrain/augur/issues/1264 still useful?

joverlee521 commented 2 months ago

Is https://github.com/nextstrain/augur/issues/1264 still useful?

Thanks for flagging @victorlin! Closed https://github.com/nextstrain/augur/issues/1264.

Then we would need to add tip_label as an allowed property for display_defaults in our auspice config schema.

Looks like this was added in https://github.com/nextstrain/augur/commit/6ce1bf70a4ff7ccfed3e3dde04f5c21c2c80c78b.

I also think export v2 should verify the tip_label field is an available node_attr or automatically add it as a node_attr if the field is available in the metadata file.

I still think this would be nice, but not prioritized.