-
Notifications
You must be signed in to change notification settings - Fork 42
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
Jgf writer simplification #1299
Jgf writer simplification #1299
Conversation
Problem: the JGF reader understands edges as containment by default, but the JGF writer outputs the value anyway even though it can be quite verbose. Suppress edge metadata for containment edges.
Problem: there are some extra branches in emit_vtx_base that could be cut down. Cut down the unnecessary branches.
3ff7d5c
to
e7e50b6
Compare
Problem: the JGF writer outputs a lot of default data for vertices. Do not output default data, let the reader infer defaults.
e7e50b6
to
3e533c5
Compare
Ok @trws there is one failing test but I'm pretty sure I know what's wrong and it's just a matter of updating some JGFs after making sure everything looks sane (which is kind of slow since the JSON isn't formatted nicely so requires a lot of manual intervention). If the code looks good to you I could probably get the tests passing in time for the release tomorrow. |
Problem: the JGF writer no longer outputs some default data, but the tests expect to find it. Change the tests.
3e533c5
to
c1af690
Compare
Thanks! Setting MWP |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1299 +/- ##
========================================
- Coverage 75.3% 75.3% -0.1%
========================================
Files 111 111
Lines 15298 15297 -1
========================================
- Hits 11530 11529 -1
Misses 3768 3768
|
Now that #1293 has made the JGF reader smarter about defaults and the ion-R utility less verbose, the JGF match writer needs similar treatment.