Open danpf opened 4 years ago
insCodeList
and altLocList
strings get dumped as null bytes: "\u0000"
. Would look better as empty string, right?
Some comments and observations:
bondResonanceList
field is not converted for the json output. On my machine I could safely drop those conversions for bondOrderList
, bondResonanceList
and secStructList
. Any reason to keep them?mmtf_tests.cpp
). At least Python and Sublime complained about the generated JSON file. So we should either drop the json-dump for the extra properties or just dump the keys.traverse.cpp
example code had a json dumper which I quickly extended to compare with the to_json
function. The advantage in traverse.cpp
is that it uses null
for data which wasn't set. This can be confusing otherwise for optional numeric fields (e.g. resolution
) which are set to std::numeric_limits<T>::max()
if they are not provided in the input MMTF file. Also traverse.cpp
does some custom pretty-printing suited for MMTF-content. On the down-side, the traverse.cpp
is an ugly C-style C++ code (my apologies since I wrote it) which was originally meant to allow diffing with the mmtf-c
library and grew beyond sustainability.to_json
function in traverse.cpp
to be able to call it from there.to_json.cpp
app is useful within tests
and I would drop it. The code is compiled anyway, so a pure "it compiles" test is not adding anything. It could be useful in examples
but the traverse.cpp
could cover that (btw: you can also call the StructureData::print
functionality from traverse.cpp
).So to sum up: If we have such a to_json
function as part of the library it should handle unset optional fields. I suggest to drop them since this already happens within GroupType objects (handled automatically via msgpack_encoders.hpp
). Also the items above (unneeded conversions, faulty JSON with extra props, unneeded to_json.cpp
) should be fixed.
I think this is reasonable to have.... I've had some interest myself in better mmtf 'diff' tools, and had a complaint or two from rosetta users about diffing abilities.
let me know what you think!
i can add an app to do this too if that's of interest