-
Notifications
You must be signed in to change notification settings - Fork 129
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
fix & test augur export for v2 (unified) JSONs #297
Comments
@jameshadfield With regard to the error in The new unified JSON schema changes how colouring is specified. In V1 JSONs So this is where the ordering problem comes from. As we've found before, though dicts technically do not have an order, it looks like through This is because in Setting this to So one solution would be to add an argument to What think you? |
@emmahodcroft It's generally unreliable to rely on the order of keys in a dictionary when crossing languages, regardless of what order they appear in when serialized to JSON. Different languages make different guarantees about preserving key ordering. I think the simplest thing would be to keep the scale as a list of lists, which is a standard and reasonable approach to cross-language ordered key-value pairs. This also allows arbitrary user-defined ordering rather than forcing an ASCII-betical sort. @jameshadfield What motivated the change from a list of lists to a dict? Could we keep it as a list of lists? |
I don't see a problem with returning to the v1 schema here (list of lists). Can't remember anything important about why I proposed turning it into a dict. |
FYI @joverlee521 and I just finished going through the
Kairsten's
|
Now working in the |
Currently
augur export
works well for v1 JSONs (i.e. meta + tree files) but is generally broken for v2 (henceforth referred to as "unified") JSON production. This issue asks to get this functionality working for unified JSONs as currently consumed by auspice on thev2
branch: https://github.com/nextstrain/auspice/tree/v2. This functionality is urgently needed in order to better test auspice v2. Note that some unified JSON functionality inaugur export
is present, however it doesn't generate JSONs which validate against the schema (see below).If auspice from the v2 branch is installed (e.g.
npm install auspice@alpha
-- see nextstrain/auspice#720 for more deatils & background) thenauspice convert
is available which converts meta+tree JSONs to the unified JSON. This may be helpful to compare augur vs auspice approaches.schema & config modifications
While there are multiple issues proposing slight revisions to the schema for the unified JSON, this issue asks it to be implemented as per the JSON schema defined here: https://github.com/nextstrain/augur/pull/277/files#diff-c021e1723b2195f3dada2595f3d70ebb. After it is working for this it'll be simple to add slight modifications as needed before the we release a new version.
As far as this issue is concerned, it's simplest to use the same (or similar) auspice config JSON. There are proposals to change this, however this discussion is best held somewhere else, with subsequent modifications added in after this issue has been closed.
User interface for
augur export
@emmahodcroft and I talked a lot about this a few weeks ago and decided that it's desirable to have command line arguments available to replace a config file for simple use cases The command line arguments are currently implemented in augur master. For complicated cases it's necessary to use a JSON config file (see above), however the ability to use a config file is not yet implemented in augur.
The text was updated successfully, but these errors were encountered: