From d0406f09a3ddf0b186867f37454065678b310299 Mon Sep 17 00:00:00 2001 From: Soeren Sonnenburg Date: Mon, 29 Apr 2013 22:23:13 +0200 Subject: [PATCH 1/2] fix json serialization (Closes: #943) --- .../serialization_complex_example.py | 19 ++++---- src/shogun/io/SerializableJsonFile.cpp | 46 ++++++++++++------- src/shogun/io/SerializableJsonReader00.cpp | 4 +- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/examples/undocumented/python_modular/serialization_complex_example.py b/examples/undocumented/python_modular/serialization_complex_example.py index fa9ba672013..b811571f8e3 100644 --- a/examples/undocumented/python_modular/serialization_complex_example.py +++ b/examples/undocumented/python_modular/serialization_complex_example.py @@ -29,6 +29,7 @@ def serialization_complex_example (num=5, dist=1, dim=10, C=2.0, width=10): feats=RealFeatures(data) #feats.io.set_loglevel(MSG_DEBUG) + #feats.io.enable_file_and_line() kernel=GaussianKernel(feats, feats, width) labels=MulticlassLabels(lab) @@ -50,9 +51,9 @@ def serialization_complex_example (num=5, dist=1, dim=10, C=2.0, width=10): status = svm.save_serializable(fstream) check_status(status,'asc') - #fstream = SerializableJsonFile("blaah.json", "w") - #status = svm.save_serializable(fstream) - #check_status(status,'json') + fstream = SerializableJsonFile("blaah.json", "w") + status = svm.save_serializable(fstream) + check_status(status,'json') fstream = SerializableXmlFile("blaah.xml", "w") status = svm.save_serializable(fstream) @@ -71,11 +72,11 @@ def serialization_complex_example (num=5, dist=1, dim=10, C=2.0, width=10): check_status(status,'asc') new_svm.train() - #fstream = SerializableJsonFile("blaah.json", "r") - #new_svm=GMNPSVM() - #status = new_svm.load_serializable(fstream) - #check_status(status,'json') - #new_svm.train() + fstream = SerializableJsonFile("blaah.json", "r") + new_svm=GMNPSVM() + status = new_svm.load_serializable(fstream) + check_status(status,'json') + new_svm.train() fstream = SerializableXmlFile("blaah.xml", "r") new_svm=GMNPSVM() @@ -85,7 +86,7 @@ def serialization_complex_example (num=5, dist=1, dim=10, C=2.0, width=10): os.unlink("blaah.h5") os.unlink("blaah.asc") - #os.unlink("blaah.json") + os.unlink("blaah.json") os.unlink("blaah.xml") return svm,new_svm diff --git a/src/shogun/io/SerializableJsonFile.cpp b/src/shogun/io/SerializableJsonFile.cpp index 6c055ae7de5..7c8f03115e8 100644 --- a/src/shogun/io/SerializableJsonFile.cpp +++ b/src/shogun/io/SerializableJsonFile.cpp @@ -123,9 +123,8 @@ CSerializableJsonFile::close() if (m_stack_stream.get_num_elements() == 1) { if (m_task == 'w' - && is_error( - json_object_to_file(m_filename, m_stack_stream.back()) - )) { + && json_object_to_file(m_filename, m_stack_stream.back())) + { SG_WARNING("Could not close file `%s' for writing!\n", m_filename); } @@ -193,7 +192,8 @@ CSerializableJsonFile::write_scalar_wrapped( return false; } - if (is_error(m_stack_stream.back())) return false; + if (is_error(m_stack_stream.back())) + return false; return true; } @@ -248,8 +248,8 @@ CSerializableJsonFile::write_stringentry_end_wrapped( json_object* array = m_stack_stream.get_element( m_stack_stream.get_num_elements() - 2); - if (is_error(json_object_array_put_idx( - array, y, m_stack_stream.back()))) return false; + if (json_object_array_put_idx( array, y, m_stack_stream.back())) + return false; pop_object(); return true; @@ -262,7 +262,9 @@ CSerializableJsonFile::write_sparse_begin_wrapped( push_object(json_object_new_object()); json_object* buf = json_object_new_array(); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; + json_object_object_add(m_stack_stream.back(), STR_KEY_SPARSE_FEATURES, buf); @@ -284,12 +286,15 @@ CSerializableJsonFile::write_sparseentry_begin_wrapped( index_t feat_index, index_t y) { json_object* buf = json_object_new_object(); - if (is_error(json_object_array_put_idx(m_stack_stream.back(), y, - buf))) return false; + if (json_object_array_put_idx(m_stack_stream.back(), y, buf)) + return false; + push_object(buf); buf = json_object_new_int(feat_index); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; + json_object_object_add(m_stack_stream.back(), STR_KEY_SPARSE_FEATINDEX, buf); @@ -340,14 +345,17 @@ CSerializableJsonFile::write_sgserializable_begin_wrapped( EPrimitiveType generic) { if (*sgserializable_name == '\0') { - push_object(NULL); return true; + push_object(NULL); + return true; } push_object(json_object_new_object()); json_object* buf; buf = json_object_new_string(sgserializable_name); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; + json_object_object_add(m_stack_stream.back(), STR_KEY_INSTANCE_NAME, buf); @@ -355,13 +363,16 @@ CSerializableJsonFile::write_sgserializable_begin_wrapped( string_t buf_str; TSGDataType::ptype_to_string(buf_str, generic, STRING_LEN); buf = json_object_new_string(buf_str); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; + json_object_object_add(m_stack_stream.back(), STR_KEY_GENERIC_NAME, buf); } buf = json_object_new_object(); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; json_object_object_add(m_stack_stream.back(), STR_KEY_INSTANCE, buf); push_object(buf); @@ -385,7 +396,8 @@ CSerializableJsonFile::write_type_begin_wrapped( const TSGDataType* type, const char* name, const char* prefix) { json_object* buf = json_object_new_object(); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; json_object_object_add(m_stack_stream.back(), name, buf); push_object(buf); @@ -393,7 +405,9 @@ CSerializableJsonFile::write_type_begin_wrapped( string_t str_buf; type->to_string(str_buf, STRING_LEN); buf = json_object_new_string(str_buf); - if (is_error(buf)) return false; + if (is_error(buf)) + return false; + json_object_object_add(m_stack_stream.back(), STR_KEY_TYPE, buf); return true; diff --git a/src/shogun/io/SerializableJsonReader00.cpp b/src/shogun/io/SerializableJsonReader00.cpp index a81b150e2d3..d63c9263579 100644 --- a/src/shogun/io/SerializableJsonReader00.cpp +++ b/src/shogun/io/SerializableJsonReader00.cpp @@ -304,8 +304,8 @@ SerializableJsonReader00::read_type_begin_wrapped( if (strcmp(str_buf, json_object_get_string(buf)) != 0) return false; - if (!m_file->get_object_any(&buf, buf_type, STR_KEY_DATA)) - return false; + // data (and so buf) can be NULL for empty objects + m_file->get_object_any(&buf, buf_type, STR_KEY_DATA); m_file->push_object(buf); return true; From c7a100aeb2357d954791c00515c0a40a627de3bc Mon Sep 17 00:00:00 2001 From: Soeren Sonnenburg Date: Mon, 29 Apr 2013 22:58:33 +0200 Subject: [PATCH 2/2] add fancy difflib based diff to determine failures on travis --- tests/integration/python_modular/tester.py | 36 +++++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/tests/integration/python_modular/tester.py b/tests/integration/python_modular/tester.py index 58ad57dd3f4..662c318b4e7 100755 --- a/tests/integration/python_modular/tester.py +++ b/tests/integration/python_modular/tester.py @@ -6,6 +6,7 @@ import filecmp import numpy import sys +import difflib from generator import setup_tests, get_fname, blacklist, get_test_mod, run_test @@ -84,6 +85,23 @@ def compare_dbg_helper(a, b, tolerance): print "b", b return False +def get_fail_string(a): + failed_string = [] + if type(a) in (tuple,list): + for i in xrange(len(a)): + failed_string.append(get_fail_string(a[i])) + elif isinstance(a, modshogun.SGObject): + failed_string.append(pickle.dumps(a)) + else: + failed_string.append(str(a)) + return failed_string + +def get_split_string(a): + strs=[] + for l in a: + strs.extend(l[0].replace('\\n','\n').splitlines()) + return strs + def tester(tests, cmp_method, tolerance, failures, missing): failed=[] @@ -112,10 +130,7 @@ def tester(tests, cmp_method, tolerance, failures, missing): print "%-60s OK" % setting_str else: if not missing: - failed_string = [] - failed_string.append(a) - failed_string.append(b) - failed.append((setting_str, failed_string)) + failed.append((setting_str, get_fail_string(a), get_fail_string(b))) print "%-60s ERROR" % setting_str except Exception, e: print setting_str, e @@ -154,6 +169,17 @@ def tester(tests, cmp_method, tolerance, failures, missing): print print "The following tests failed!" for f in failed: - print "\t", f + print "\t", f[0] + expected=get_split_string(f[1]) + got=get_split_string(f[2]) + print "=== EXPECTED ==========" + #import pdb + #pdb.set_trace() + print '\n'.join(expected) + print "=== GOT ===============" + print '\n'.join(got) + print "====DIFF================" + print '\n'.join(difflib.unified_diff(expected, got, fromfile='expected', tofile='got')) + print "====EOT================" sys.exit(1) sys.exit(0)