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

BUG: Export complains if node data json contains only empty dicts for nodes and branches #1215

Closed
corneliusroemer opened this issue May 15, 2023 · 0 comments · Fixed by #1214
Labels
bug Something isn't working

Comments

@corneliusroemer
Copy link
Member

Current Behavior

Passing an uninformative node_data.json to export v2 raises an error:

ERROR: results/europe/rbd_levels.json did not contain either `nodes` or `branches`. Please check the formatting of this JSON!

Expected behavior

It is ok to pass an empty node data json to export, no error, at most a warning

How to reproduce

Steps to reproduce the current behavior:

  1. gh repo clone nextstrain/ncov
  2. cd ncov
  3. nextstrain build --ambient . all_regions -j 2 --profile nextstrain_profiles/nextstrain-ci -F
  4. Observe error

Possible solution

The error is new in augur v22.0.0 due to new validation introduced in #728

Removing this new validation fixes this bug

Resolved by #1214

Your environment: if running Nextstrain locally

  • Operating system: macOS 13.3.1 (arm)
  • Version: augur 22.0.0
@corneliusroemer corneliusroemer added the bug Something isn't working label May 15, 2023
@corneliusroemer corneliusroemer linked a pull request May 15, 2023 that will close this issue
2 tasks
corneliusroemer added a commit that referenced this issue May 15, 2023
Resolves #1215

Warn instead error when no nodes in a node data json, fixing issue introduced recently in PR #728

In PR #728, extra node data validation was introduced. In particular, files without information for either `nodes` or `branches` caused erroring.

This is problematic for test scripts that may produce empty node data in test cases.

This PR removes the eager validation. In the future we could reintroduce it as a warning.
And possibly an error but with opt-out.

This type of node data json was previously errored on by augur export, it is now accepted again:

```json
{
  "nodes": {},
  "rbd_level_details": {}
}
```

<!-- Start typing the name of a related issue and GitHub will auto-suggest the issue number for you.  -->
Fixes the ncov pathogen-CI issue: nextstrain/conda-base#27 (comment)

What steps should be taken to test the changes you've proposed?
If you added or changed behavior in the codebase, did you update the tests, or do you need help with this?

- [x] nextstrain/conda-base#27 (comment) is fixed, export now accepts empty nodes dicts again
corneliusroemer added a commit that referenced this issue May 15, 2023
Inspired by bug #1215 which we only noticed after release but which could have been noticed had we run pathogen ci for ncov within Augur's CI.

Resolves #1216
corneliusroemer added a commit that referenced this issue May 16, 2023
Inspired by bug #1215 which we only noticed after release but which could have been noticed had we run pathogen ci for ncov within Augur's CI.

Resolves #1216

Use ${{github.ref}} for pip install git

Try to install correct augur version

revert shell default

Revert formatting changes

reinsert the new part

Use nextstrain-base for DRYness

Cache environment for reduced ci usage

Don't cache packages, only env
corneliusroemer added a commit that referenced this issue May 16, 2023
Inspired by bug #1215 which we only noticed after release but which could have been noticed had we run pathogen ci for ncov within Augur's CI.

Resolves #1216

Use ${{github.ref}} for pip install git

Try to install correct augur version

revert shell default

Revert formatting changes

reinsert the new part

Use nextstrain-base for DRYness

Cache environment for reduced ci usage

Don't cache packages, only env
corneliusroemer added a commit that referenced this issue May 16, 2023
Inspired by bug #1215 which we only noticed after release but which could have been noticed had we run pathogen ci for ncov within Augur's CI.

Resolves #1216

Use ${{github.ref}} for pip install git

Try to install correct augur version

revert shell default

Revert formatting changes

reinsert the new part

Use nextstrain-base for DRYness

Cache environment for reduced ci usage

Don't cache packages, only env
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant