Skip to content

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Feb 26, 2021

It turns out we're missing a large chunk of tuning, since the JSON file forces all keys to be string, but we expect keys to be semi-rich types (for rich filtering). Compromise instead and coerce some types to string immediately and parse-eval other strings back to basic types (string, number, and tuples thereof).

@codecov-io
Copy link

Codecov Report

Merging #193 (c993884) into master (2e2e6ef) will decrease coverage by 2.05%.
The diff coverage is 70.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   87.77%   85.71%   -2.06%     
==========================================
  Files           6        6              
  Lines         687      714      +27     
==========================================
+ Hits          603      612       +9     
- Misses         84      102      +18     
Impacted Files Coverage Δ
src/serialization.jl 84.81% <64.70%> (-15.19%) ⬇️
src/groups.jl 93.37% <77.77%> (-3.15%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e2e6ef...c993884. Read the comment docs.

@vtjnash
Copy link
Member Author

vtjnash commented Feb 26, 2021

The de-JSON recover-y should also incidentally fix the awkward doubled-quoting of the reports in Nanosoldier such as https://github.com/JuliaCI/NanosoldierReports/blob/master/benchmark/by_date/2021-02/17/report.md

@vtjnash vtjnash requested a review from ararslan March 2, 2021 20:32
Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense I think, though it's been ages since I've thought about the internals here.

@vtjnash vtjnash merged commit a385dcc into master Mar 4, 2021
@vtjnash vtjnash deleted the jn/repr-keys branch March 4, 2021 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants