export v2: Add --metadata-columns option by joverlee521 · Pull Request #1384 · nextstrain/augur

joverlee521 · 2024-01-06T01:09:25Z

Description of proposed changes

Allows users to specify additional metadata columns to export as
node attributes that are not specified as coloring options via
the existing --color-by-metadata option or the Auspice config JSON.
This allows the metadata columns to be visible in the tree in Auspice
without polluting the color-by options.

Related issue(s)

Resolves #1383

Checklist

Checks pass
If making user-facing changes, add a message in CHANGES.md summarizing the changes in this PR

codecov · 2024-01-06T01:18:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (abc86a8) 66.71% compared to head (32c9740) 66.96%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1384      +/-   ##
==========================================
+ Coverage   66.71%   66.96%   +0.24%     
==========================================
  Files          69       69              
  Lines        7322     7341      +19     
  Branches     1799     1804       +5     
==========================================
+ Hits         4885     4916      +31     
+ Misses       2169     2154      -15     
- Partials      268      271       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jameshadfield · 2024-01-07T20:23:07Z

Nice one Jover - the overall aim of this work is something I've wanted for a long time. I haven't tested the code, just skimmed through.

These columns will not be used as coloring options in Auspice but will be visible in the tree

Here do you mean visible in the modal when (shift-)clicking on a branch/tip? It'd be good to surface them as sidebar filtering options too, but that'll take an Auspice change I think. My understanding of the linked issue (and from discussions with others) is that datasets are adding unnecessary colorings in order to expose metadata as filtering options.

joverlee521 · 2024-01-08T19:17:23Z

Here do you mean visible in the modal when (shift-)clicking on a branch/tip?

Yes, I mean they will only be visible on the modal on click.

It'd be good to surface them as sidebar filtering options too, but that'll take an Auspice change I think. My understanding of the linked issue (and from discussions with others) is that datasets are adding unnecessary colorings in order to expose metadata as filtering options.

Yup, I think this will require changes in Auspice. My understanding is that currently filters must be defined in the Auspice JSON under meta.filters. Even colorings are not automatically available as filters (nextstrain/auspice#1251).

victorlin

Functionality is great! Suggestions below are non-blocking (except changelog request).

jameshadfield

Allows users to specify additional metadata columns to export as node attributes that are not specified as coloring options via the existing --color-by-metadata option or the Auspice config JSON. This allows the metadata columns to be visible in the tree in Auspice without polluting the color-by options.

This behaviour is already possible by specifying the (TSV metadata column names) in the auspice config's filters array. There are side effects beyond those implemented in this PR, as each key in filters will be shown as a (collapsed) section in the footer, but the (big) upside is that the metadata is searchable.

I want to change the general behaviour here a bit, but I think we are better to sketch the big direction out first before merging any PRs. I'm envisioning something like the following, but it's not been completely thought through:

Auspice uses all node_attrs as sidebar filtering options (and maybe make branch labels searchable too, but that's complicated).
The filters array (in the dataset JSON) is reserved for the items listed in the footer. I see this functionality as less important now that we have sidebar filtering.
We have some way of exporting metadata (TSV) fields without them being colourings. Maybe that's this PR?

But maybe just doing (1) is enough? Thoughts?

jameshadfield · 2024-01-29T01:20:13Z

                                 help="delimiters to accept when reading a metadata file. Only one delimiter will be inferred.")
    optional_inputs.add_argument('--metadata-id-columns', default=DEFAULT_ID_COLUMNS, nargs="+",
                                 help="names of possible metadata columns containing identifier information, ordered by priority. Only one ID column will be inferred.")
+    optional_inputs.add_argument('--metadata-columns', nargs="+",


Any functionality we support via command line arguments should be available via the auspice-config JSON as well, as the former is a subset of the latter's capabilities.

Hmm, this flag relates to the metadata input/export but not really Auspice control settings. Not sure this fits in the auspice-config JSON?

I see it as controlling what data is exported, similar to filters & colours. The (original) intention when we started adding command line arguments for options in the config JSON was for auspice-config to be all you need to control what data the JSON contains.

Okay, that makes sense. I'll move up the flag to be part of the config args group too then.

Auspice config schema update and changes to support the new property added in bdb2b2c

joverlee521 · 2024-01-29T21:53:36Z

I want to change the general behaviour here a bit, but I think we are better to sketch the big direction out first before merging any PRs.

Auspice uses all node_attrs as sidebar filtering options (and maybe make branch labels searchable too, but that's complicated).

The filters array (in the dataset JSON) is reserved for the items listed in the footer. I see this functionality as less important now that we have sidebar filtering.

We have some way of exporting metadata (TSV) fields without them being colourings. Maybe that's this PR?

But maybe just doing (1) is enough? Thoughts?

I think (1) is definitely needed. I think this will require some design on filtering for continuous value fields?

On (2), I think we can even deprecate the footer filters. I definitely only use the sidebar filtering and have not looked at footer filters in a while.

I think even if we implement (1), we still need some way to support (3). I think it would be nice to fully detangle exported metadata from coloring options. I would want to be more explicit about what metadata fields are exported instead of letting it stay a side effect of other Auspice configurations.

Allows users to specify additional metadata columns to export as node attributes that are not specified as coloring options via the existing `--color-by-metadata` option or the Auspice config JSON. This allows the metadata columns to be visible in the tree in Auspice without polluting the color-by options.

@victorlin

Suggested by @victorlin in review.

joverlee521 · 2024-02-06T18:02:29Z

Rebased to fix merge conflicts in changelog.

joverlee521 · 2024-02-06T18:23:23Z

@jameshadfield since (1) was implemented in nextstrain/auspice#1743, do you still have feedback here?

jameshadfield · 2024-02-06T21:23:35Z

@jameshadfield since (1) was implemented in nextstrain/auspice#1743, do you still have feedback here?

On (2), I think we can even deprecate the footer filters. I definitely only use the sidebar filtering and have not looked at footer filters in a while.

My interpretation of the deprecation here was to deprecate the auspice UI, but keep the augur export filters setting and use this as the way to add metadata that won't be a coloring. As in "what data would you like to include for filtering in auspice". But would you prefer to add --metadata-columns and remove --filters entirely?

(If we do add --metadata-columns, I do think it needs to be a setting in the auspice-config as well.)

joverlee521 · 2024-02-07T00:13:19Z

My interpretation of the deprecation here was to deprecate the auspice UI, but keep the augur export filters setting and use this as the way to add metadata that won't be a coloring. As in "what data would you like to include for filtering in auspice". But would you prefer to add --metadata-columns and remove --filters entirely?

(If we do add --metadata-columns, I do think it needs to be a setting in the auspice-config as well.)

I'd prefer to add --metadata-columns. The --filters flag does not currently exist, so this would just be deprecating the filters key in the auspice config JSON.

For me,--filters does not tell me if the data is coming from the metadata TSV. I'd be confused about whether or not I need to include node-data JSON fields to use them as filters as well.

@jameshadfield

Since the new `--metadata-columns` flag controls what data is available in the final Auspice JSON, it should have a matching option in the Auspice config JSON setting. This also moves the flag to the "DISPLAY CONFIGURATION" group to match the other Auspice config controlled settings. As per the existing pattern, the command line flag overrules what is set in the config file. Based on feedback from @jameshadfield #1384 (comment)

huddlej · 2024-02-07T23:40:03Z

On (2), I think we can even deprecate the footer filters. I definitely only use the sidebar filtering and have not looked at footer filters in a while.

I'd vote to please keep the footer filters, if possible. I still use them regularly and find them helpful for quickly picking from a longer list of options (e.g., year-month values for flu, clades, regions, etc.). It's generally been helpful to have a way to see in a single view what the filter fields are and the specific values within each field.

jameshadfield

I haven't re-reviewed everything but the changes in bdb2b2c look good by inspection

joverlee521 · 2024-02-08T00:45:46Z

I'd vote to please keep the footer filters, if possible. I still use them regularly and find them helpful for quickly picking from a longer list of options (e.g., year-month values for flu, clades, regions, etc.). It's generally been helpful to have a way to see in a single view what the filter fields are and the specific values within each field.

@huddlej No worries, this PR doesn't touch the footer filters! Although this reminded me that the default footer filters will likely only be a subset of available filter fields since it currently does not include the additional metadata columns!

augur/augur/export_v2.py

Lines 539 to 542 in 4bf72f8

    
           # if not specified in the config, include all boolean and categorical colorbys 
        
           potentials = {coloring["key"] for coloring in data_json['meta']["colorings"] 
        
                         if coloring["type"] != "continuous" and coloring["key"] != 'gt'} 
        
           data_json['meta']['filters'] = list(potentials)

joverlee521 · 2024-02-08T17:54:21Z

Merging this PR as-is, will update the footer filters separately (if desired).

Fixing changelog update made in #1384. The PR was actually released as part of v24.2.0, not v 24.1.0.

jameshadfield reviewed Jan 7, 2024

View reviewed changes

Comment thread augur/export_v2.py Outdated

joverlee521 mentioned this pull request Jan 8, 2024

Sidebar filtering options should include all node/branch attrs nextstrain/auspice#1251

Closed

joverlee521 mentioned this pull request Jan 18, 2024

Persephone nextstrain/zika#28

Merged

1 task

victorlin approved these changes Jan 26, 2024

View reviewed changes

jameshadfield self-requested a review January 26, 2024 01:47

joverlee521 mentioned this pull request Jan 26, 2024

Add jq to dev dependency #1399

Open

joverlee521 force-pushed the export-additional-metadata branch from 6b1dbf6 to 4bf28ec Compare January 27, 2024 00:51

joverlee521 mentioned this pull request Jan 27, 2024

Update metadata section in Augur to Auspice nextstrain/docs.nextstrain.org#184

Open

jameshadfield requested changes Jan 29, 2024

View reviewed changes

joverlee521 mentioned this pull request Jan 29, 2024

Fix RBD level coloring and filter nextstrain/ncov#1094

Merged

5 tasks

joverlee521 added 3 commits February 6, 2024 09:55

export_v2: remove excess whitespace

90fe0b1

export_v2: Align wording in metadata warnings

54a351e

Suggested by @victorlin in review.

joverlee521 force-pushed the export-additional-metadata branch from 4bf28ec to 1b83e78 Compare February 6, 2024 17:57

joverlee521 added 2 commits February 7, 2024 13:37

Update changelog

32c9740

joverlee521 force-pushed the export-additional-metadata branch from 1b83e78 to 32c9740 Compare February 7, 2024 21:52

joverlee521 requested a review from a team February 7, 2024 21:53

jameshadfield approved these changes Feb 7, 2024

View reviewed changes

joverlee521 merged commit e4353b4 into master Feb 8, 2024

joverlee521 deleted the export-additional-metadata branch February 8, 2024 17:54

joverlee521 mentioned this pull request Feb 8, 2024

[export] Mismatch between default footer filters and Auspice sidebar filters #1409

Open

joverlee521 added a commit that referenced this pull request Feb 16, 2024

Update changelog

5b68d8e

Fixing changelog update made in #1384. The PR was actually released as part of v24.2.0, not v 24.1.0.

joverlee521 mentioned this pull request Feb 16, 2024

Update changelog #1419

Merged

Conversation

joverlee521 commented Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of proposed changes

Related issue(s)

Checklist

Uh oh!

codecov bot commented Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jameshadfield commented Jan 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joverlee521 commented Jan 8, 2024

Uh oh!

victorlin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jameshadfield left a comment

Choose a reason for hiding this comment

Uh oh!

jameshadfield Jan 29, 2024

Choose a reason for hiding this comment

Uh oh!

joverlee521 Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

jameshadfield Feb 6, 2024

Choose a reason for hiding this comment

Uh oh!

joverlee521 Feb 7, 2024

Choose a reason for hiding this comment

Uh oh!

joverlee521 Feb 7, 2024

Choose a reason for hiding this comment

Uh oh!

joverlee521 commented Jan 29, 2024

Uh oh!

joverlee521 commented Feb 6, 2024

Uh oh!

joverlee521 commented Feb 6, 2024

Uh oh!

jameshadfield commented Feb 6, 2024

Uh oh!

joverlee521 commented Feb 7, 2024

Uh oh!

huddlej commented Feb 7, 2024

Uh oh!

jameshadfield left a comment

Choose a reason for hiding this comment

Uh oh!

joverlee521 commented Feb 8, 2024

Uh oh!

joverlee521 commented Feb 8, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

joverlee521 commented Jan 6, 2024 •

edited

Loading

codecov bot commented Jan 6, 2024 •

edited

Loading

jameshadfield commented Jan 7, 2024 •

edited

Loading