From d957d5e8f6e8bae0be785ab5124bb30ae85f8737 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sat, 4 Feb 2023 14:52:28 -0800 Subject: [PATCH] Clean up code and docs --- gtsam/discrete/SignatureParser.cpp | 10 ++- gtsam/discrete/SignatureParser.h | 53 +++++++++---- gtsam/discrete/tests/testSignatureParser.cpp | 79 +++++++++++--------- 3 files changed, 90 insertions(+), 52 deletions(-) diff --git a/gtsam/discrete/SignatureParser.cpp b/gtsam/discrete/SignatureParser.cpp index cf073595b..023fea5bf 100644 --- a/gtsam/discrete/SignatureParser.cpp +++ b/gtsam/discrete/SignatureParser.cpp @@ -18,7 +18,8 @@ inline static SignatureParser::Table ParseAnd() { return {ParseFalseRow(), ParseFalseRow(), ParseFalseRow(), ParseTrueRow()}; } -bool static ParseConditional(const std::string& token, std::vector& row) { +bool static ParseConditional(const std::string& token, + std::vector& row) { // Expect something like a/b/c std::istringstream iss2(token); try { @@ -35,7 +36,8 @@ bool static ParseConditional(const std::string& token, std::vector& row) return true; } -void static ParseConditionalTable(const std::vector& tokens, SignatureParser::Table& table) { +void static ParseConditionalTable(const std::vector& tokens, + SignatureParser::Table& table) { // loop over the words // for each word, split it into doubles using a stringstream for (const auto& word : tokens) { @@ -105,8 +107,8 @@ bool SignatureParser::parse(const std::string& str, Table& table) { if (table.empty()) { return false; } - // the boost::phoenix parser did not return an error if we could not fully parse a string - // it just returned whatever it could parse + // the boost::phoenix parser did not return an error if we could not fully + // parse a string it just returned whatever it could parse return true; } } // namespace gtsam diff --git a/gtsam/discrete/SignatureParser.h b/gtsam/discrete/SignatureParser.h index a4a9f05f5..98904ec51 100644 --- a/gtsam/discrete/SignatureParser.h +++ b/gtsam/discrete/SignatureParser.h @@ -1,18 +1,19 @@ +/* ---------------------------------------------------------------------------- + + * GTSAM Copyright 2010, Georgia Tech Research Corporation, + * Atlanta, Georgia 30332-0415 + * All Rights Reserved + * Authors: Frank Dellaert, et al. (see THANKS for the full author list) + + * See LICENSE for the license information + + * -------------------------------------------------------------------------- */ + /** - * This is a simple parser that replaces the boost spirit parser. It is - * meant to parse strings like "1/1 2/3 1/4". Every word of the form "a/b/c/..." - * should be parsed as a row, and the rows should be stored in a table. - * The elements of the row will be doubles. - * The table is a vector of rows. The row is a vector of doubles. - * The parser should be able to parse the following strings: - * "1/1 2/3 1/4": {{1,1},{2,3},{1,4}} - * "1/1 2/3 1/4 1/1 2/3 1/4" : {{1,1},{2,3},{1,4},{1,1},{2,3},{1,4}} - * "1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3" : {{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3}} - * If the string has unparseable elements, the parser should parse whatever it can - * "1/2 sdf" : {{1,2}} - * It should return false if the string is empty. - * "": false - * We should return false if the rows are not of the same size. + * @file SignatureParser.h + * @brief Parser for conditional distribution signatures. + * @author Kartik Arcot + * @date January 2023 */ #pragma once @@ -21,6 +22,30 @@ #include namespace gtsam { +/** + * @brief A simple parser that replaces the boost spirit parser. + * + * It is meant to parse strings like "1/1 2/3 1/4". Every word of the form + * "a/b/c/..." is parsed as a row, and the rows are stored in a table. + * + * A `Row` is a vector of doubles, and a `Table` is a vector of rows. + * + * Examples: the parser is able to parse the following strings: + * "1/1 2/3 1/4": + * {{1,1},{2,3},{1,4}} + * "1/1 2/3 1/4 1/1 2/3 1/4" : + * {{1,1},{2,3},{1,4},{1,1},{2,3},{1,4}} + * "1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3 2/3/4 1/2/3" : + * {{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3},{2,3,4},{1,2,3}} + * + * If the string has un-parsable elements, should parse whatever it can: + * "1/2 sdf" : {{1,2}} + * + * It should return false if the string is empty: + * "": false + * + * We should return false if the rows are not of the same size. + */ namespace SignatureParser { typedef std::vector Row; typedef std::vector Table; diff --git a/gtsam/discrete/tests/testSignatureParser.cpp b/gtsam/discrete/tests/testSignatureParser.cpp index 017b8e70c..99acd56be 100644 --- a/gtsam/discrete/tests/testSignatureParser.cpp +++ b/gtsam/discrete/tests/testSignatureParser.cpp @@ -3,12 +3,16 @@ * @file testSimpleParser.cpp */ +#include #include #include -#include -bool compare_tables(const gtsam::SignatureParser::Table& table1, - const gtsam::SignatureParser::Table& table2) { +using namespace gtsam; + +/* ************************************************************************* */ +// Simple test case +bool compareTables(const SignatureParser::Table& table1, + const SignatureParser::Table& table2) { if (table1.size() != table2.size()) { return false; } @@ -25,72 +29,79 @@ bool compare_tables(const gtsam::SignatureParser::Table& table1, return true; } +/* ************************************************************************* */ // Simple test case -TEST(SimpleParser, simple) { - gtsam::SignatureParser::Table table, expectedTable; +TEST(SimpleParser, Simple) { + SignatureParser::Table table, expectedTable; expectedTable = {{1, 1}, {2, 3}, {1, 4}}; - bool ret = gtsam::SignatureParser::parse("1/1 2/3 1/4", table); + bool ret = SignatureParser::parse("1/1 2/3 1/4", table); EXPECT(ret); // compare the tables - EXPECT(compare_tables(table, expectedTable)); + EXPECT(compareTables(table, expectedTable)); } +/* ************************************************************************* */ // Test case with each row having 3 elements -TEST(SimpleParser, three_elements) { - gtsam::SignatureParser::Table table, expectedTable; +TEST(SimpleParser, ThreeElements) { + SignatureParser::Table table, expectedTable; expectedTable = {{1, 1, 1}, {2, 3, 2}, {1, 4, 3}}; - bool ret = gtsam::SignatureParser::parse("1/1/1 2/3/2 1/4/3", table); + bool ret = SignatureParser::parse("1/1/1 2/3/2 1/4/3", table); EXPECT(ret); // compare the tables - EXPECT(compare_tables(table, expectedTable)); + EXPECT(compareTables(table, expectedTable)); } -// A test case to check if we can parse a signarue with 'T' and 'F' -TEST(SimpleParser, TandF) { - gtsam::SignatureParser::Table table, expectedTable; - expectedTable = {{1,0}, {1,0}, {1,0}, {0,1}}; - bool ret = gtsam::SignatureParser::parse("F F F T", table); +/* ************************************************************************* */ +// A test case to check if we can parse a signature with 'T' and 'F' +TEST(SimpleParser, TAndF) { + SignatureParser::Table table, expectedTable; + expectedTable = {{1, 0}, {1, 0}, {1, 0}, {0, 1}}; + bool ret = SignatureParser::parse("F F F T", table); EXPECT(ret); // compare the tables - EXPECT(compare_tables(table, expectedTable)); + EXPECT(compareTables(table, expectedTable)); } +/* ************************************************************************* */ // A test to parse {F F F 1} TEST(SimpleParser, FFF1) { - gtsam::SignatureParser::Table table, expectedTable; - expectedTable = {{1,0}, {1,0}, {1,0}}; + SignatureParser::Table table, expectedTable; + expectedTable = {{1, 0}, {1, 0}, {1, 0}}; // should ignore the last 1 - bool ret = gtsam::SignatureParser::parse("F F F 1", table); + bool ret = SignatureParser::parse("F F F 1", table); EXPECT(ret); // compare the tables - EXPECT(compare_tables(table, expectedTable)); + EXPECT(compareTables(table, expectedTable)); } +/* ************************************************************************* */ // Expect false if the string is empty -TEST(SimpleParser, empty_string) { - gtsam::SignatureParser::Table table; - bool ret = gtsam::SignatureParser::parse("", table); +TEST(SimpleParser, emptyString) { + SignatureParser::Table table; + bool ret = SignatureParser::parse("", table); EXPECT(!ret); } - -// Expect false if jibberish -TEST(SimpleParser, jibberish) { - gtsam::SignatureParser::Table table; - bool ret = gtsam::SignatureParser::parse("sdf 22/3", table); +/* ************************************************************************* */ +// Expect false if gibberish +TEST(SimpleParser, Gibberish) { + SignatureParser::Table table; + bool ret = SignatureParser::parse("sdf 22/3", table); EXPECT(!ret); } -// If jibberish is in the middle, it should still parse the rest -TEST(SimpleParser, jibberish_in_middle) { - gtsam::SignatureParser::Table table, expectedTable; +// If Gibberish is in the middle, it should still parse the rest +TEST(SimpleParser, GibberishInMiddle) { + SignatureParser::Table table, expectedTable; expectedTable = {{1, 1}, {2, 3}}; - bool ret = gtsam::SignatureParser::parse("1/1 2/3 sdf 1/4", table); + bool ret = SignatureParser::parse("1/1 2/3 sdf 1/4", table); EXPECT(ret); // compare the tables - EXPECT(compare_tables(table, expectedTable)); + EXPECT(compareTables(table, expectedTable)); } +/* ************************************************************************* */ + int main() { TestResult tr; return TestRegistry::runAllTests(tr);