fix: When JSON dump with sort_key fails, retry without sorting

[corneliusroemer]

Resolves #1086, #1342, #1822

Description of proposed changes

In refine, I got a TypeError when a node name is None. The reason json.dump errors is that we sort on keys before dumping, and None can't be sorted.

augur/augur/utils.py

Line 129 in e330d39

json.dump(data, handle, indent=indent, sort_keys=sort_keys, cls=AugurJSONEncoder)

This PR makes it so that we:

• retry the dump without sort when dump with sort failed with an error message indicative of sort being the cause of the TypeError
• report a warning to the user that a node might be missing a name

Related issue(s)

I've encountered this error message at least 3 times over the last 3 years, making 3 separate issues (as I couldn't find the duplicates). I'm not alone though, another user commented they encountered the same error message: #1342 (comment)

Manual testing

It fixes the uncaught error for me:

Validating schema of 'builds/BA.2.86/muts.tmp'...
WARNING: When writing JSON, could not sort JSON keys. A tree node is likely missing a name. Retrying without sorting keys.
ancestral mutations written to builds/BA.2.86/muts.tmp

Checklist

• Automated checks pass
• Check if you need to add a changelog message
• Check if you need to add tests
• Check if you need to update docs

[codecov]

Codecov Report

Attention: Patch coverage is 61.11111% with 7 lines in your changes missing coverage. Please review.

Project coverage is 73.58%. Comparing base (e330d39) to head (d3f63af).
Report is 65 commits behind head on master.

Files with missing lines	Patch %	Lines
augur/utils.py	61.11%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1823      +/-   ##
==========================================
- Coverage   73.63%   73.58%   -0.06%     
==========================================
  Files          81       81              
  Lines        8641     8651      +10     
  Branches     1765     1766       +1     
==========================================
+ Hits         6363     6366       +3     
- Misses       1976     1983       +7     
  Partials      302      302              

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[joverlee521]

write_json is not specific to tree node JSONs and the TypeError can also be due to other types (e.g. str vs int), so this hint might be misleading/confusing.

How about just reporting the underlying error:

Suggested change

	print("WARNING: When writing JSON, could not sort JSON keys. A tree node is likely missing a name. Retrying without sorting keys.", file=sys.stderr)
	print(f"WARNING: When writing JSON, could not sort JSON keys due to the error: {e}. Retrying without sorting keys.", file=sys.stderr)

[victorlin]

You explained the underlying reason nicely in #1342 (comment). Based on that, should it error instead? If it warns and continues, the --output-node-data will have a "null" entry, which does not seem very useful.

    "null": {
      "branch_length": 0
    },

[victorlin]

Can you revert the unnecessary diffs or propose them in a separate commit/PR?