From d73509730858bccc00cd95d2d2a689971e48591e Mon Sep 17 00:00:00 2001
From: Andrew Bell <andrew.bell.ia@gmail.com>
Date: Fri, 23 Mar 2018 15:05:42 +0100
Subject: [PATCH] Add support for individual dimension precisions. Close #1869

---
 doc/stages/writers.text.rst     | 16 ++++--
 io/TextWriter.cpp               | 89 ++++++++++++++++++++++++++-------
 io/TextWriter.hpp               | 10 +++-
 test/unit/io/TextWriterTest.cpp | 49 ++++++++++++++++++
 4 files changed, 141 insertions(+), 23 deletions(-)

diff --git a/doc/stages/writers.text.rst b/doc/stages/writers.text.rst
index 4fefa14677..c51608b3d3 100644
--- a/doc/stages/writers.text.rst
+++ b/doc/stages/writers.text.rst
@@ -42,14 +42,24 @@ filename
 format
   Output format to use. One of "geojson" or "csv". [Default: **csv**]
 
+precision
+  Decimal Precision for output of values. This can be overridden for
+  individual dimensions using the **order** option. [Default: 3]
+
 order
-  Comma-separated list of dimension names, giving the desired column order in the output file, for example "X,Y,Z,Red,Green,Blue". [Default: none]
+  Comma-separated list of dimension names in the desired output order.
+  For example "X,Y,Z,Red,Green,Blue". Dimension names
+  can optionally be followed with a colon (':') and an integer to indicate the
+  precision to use for output. Ex: "X:3, Y:5,Z:0" If no precision is specified
+  the value provided with the **precision** option is used. [Default: none]
 
 keep_unspecified
-  Should we output any fields that are not specified in the dimension order? [Default: **true**]
+  If true, writes all dimensions.  Dimensions specified with the **order**
+  option precede those not specified. [Default: **true**]
 
 jscallback
-  When producing GeoJSON, the callback allows you to wrap the data in a function, so the output can be evaluated in a <script> tag.
+  When producing GeoJSON, the callback allows you to wrap the data in
+  a function, so the output can be evaluated in a <script> tag.
 
 quote_header
   When producing CSV, should the column header named by quoted? [Default: **true**]
diff --git a/io/TextWriter.cpp b/io/TextWriter.cpp
index 4a96764329..679f003b2e 100644
--- a/io/TextWriter.cpp
+++ b/io/TextWriter.cpp
@@ -99,30 +99,70 @@ void TextWriter::initialize(PointTableRef table)
 }
 
 
+TextWriter::DimSpec TextWriter::extractDim(std::string dim, PointTableRef table)
+{
+    Utils::trim(dim);
+
+    size_t precision(0);
+    StringList s = Utils::split(dim, ':');
+    if (s.size() == 1)
+        precision = m_precision;
+    else if (s.size() == 2)
+    {
+        try
+        {
+            size_t pos;
+            int i = std::stoi(s[1], &pos);
+            if (i < 0 || pos != s[1].size())
+                throw pdal_error("Dummy");  // Throw to be caught below.
+            precision = static_cast<size_t>(i);
+        }
+        catch (...)
+        {
+            throwError("Can't convert dimension precision for '" + dim +
+                "'.");
+        }
+    }
+    else
+        throwError("Invalid dimension specification '" + dim + "'.");
+    Dimension::Id d = table.layout()->findDim(s[0]);
+    if (d == Dimension::Id::Unknown)
+        throwError("Dimension not found with name '" + dim + "'.");
+    return { d, precision };
+}
+
+
+bool TextWriter::findDim(Dimension::Id id, DimSpec& ds)
+{
+    auto it = std::find_if(m_dims.begin(), m_dims.end(),
+        [id](const DimSpec& tds){ return tds.id == id; });
+    if (it == m_dims.end())
+        return false;
+    ds = *it;
+    return true;
+}
+
 void TextWriter::ready(PointTableRef table)
 {
-    m_stream->precision(m_precision);
     *m_stream << std::fixed;
 
     // Find the dimensions listed and put them on the id list.
     StringList dimNames = Utils::split2(m_dimOrder, ',');
+
     for (std::string dim : dimNames)
-    {
-        Utils::trim(dim);
-        Dimension::Id d = table.layout()->findDim(dim);
-        if (d == Dimension::Id::Unknown)
-            throwError("Dimension not found with name '" + dim + "'.");
-        m_dims.push_back(d);
-    }
+        m_dims.push_back(extractDim(dim, table));
 
     // Add the rest of the dimensions to the list if we're doing that.
     // Yes, this isn't efficient when, but it's simple.
     if (m_dimOrder.empty() || m_writeAllDims)
     {
         Dimension::IdList all = table.layout()->dims();
-        for (auto di = all.begin(); di != all.end(); ++di)
-            if (!Utils::contains(m_dims, *di))
-                m_dims.push_back(*di);
+        for (auto id : all)
+        {
+            DimSpec ds { id, static_cast<size_t>(m_precision) };
+            if (!findDim(id, ds))
+                m_dims.push_back(ds);
+        }
     }
 
     if (!m_writeHeader)
@@ -172,9 +212,9 @@ void TextWriter::writeCSVHeader(PointTableRef table)
             *m_stream << m_delimiter;
 
         if (m_quoteHeader)
-            *m_stream << "\"" << layout->dimName(*di) << "\"";
+            *m_stream << "\"" << layout->dimName(di->id) << "\"";
         else
-            *m_stream << layout->dimName(*di);
+            *m_stream << layout->dimName(di->id);
     }
     *m_stream << m_newline;
 }
@@ -187,7 +227,8 @@ void TextWriter::writeCSVBuffer(const PointViewPtr view)
         {
             if (di != m_dims.begin())
                 *m_stream << m_delimiter;
-            *m_stream << view->getFieldAs<double>(*di, idx);
+            m_stream->precision(di->precision);
+            *m_stream << view->getFieldAs<double>(di->id, idx);
         }
         *m_stream << m_newline;
     }
@@ -197,6 +238,14 @@ void TextWriter::writeGeoJSONBuffer(const PointViewPtr view)
 {
     using namespace Dimension;
 
+    auto write = [this](Dimension::Id id, PointId idx, const PointViewPtr view)
+    {
+        DimSpec ds;
+        size_t prec = findDim(id, ds) ? ds.precision : m_precision;
+        m_stream->precision(prec);
+        *m_stream << view->getFieldAs<double>(id, idx);
+    };
+
     for (PointId idx = 0; idx < view->size(); ++idx)
     {
         if (idx)
@@ -204,9 +253,10 @@ void TextWriter::writeGeoJSONBuffer(const PointViewPtr view)
 
         *m_stream << "{ \"type\":\"Feature\",\"geometry\": "
             "{ \"type\": \"Point\", \"coordinates\": [";
-        *m_stream << view->getFieldAs<double>(Id::X, idx) << ",";
-        *m_stream << view->getFieldAs<double>(Id::Y, idx) << ",";
-        *m_stream << view->getFieldAs<double>(Id::Z, idx) << "]},";
+
+        write(Id::X, idx, view); *m_stream << ",";
+        write(Id::Y, idx, view); *m_stream << ",";
+        write(Id::Z, idx, view); *m_stream << "]},";
 
         *m_stream << "\"properties\": {";
 
@@ -215,9 +265,10 @@ void TextWriter::writeGeoJSONBuffer(const PointViewPtr view)
             if (di != m_dims.begin())
                 *m_stream << ",";
 
-            *m_stream << "\"" << view->dimName(*di) << "\":";
+            *m_stream << "\"" << view->dimName(di->id) << "\":";
             *m_stream << "\"";
-            *m_stream << view->getFieldAs<double>(*di, idx);
+            m_stream->precision(di->precision);
+            *m_stream << view->getFieldAs<double>(di->id, idx);
             *m_stream <<"\"";
         }
         *m_stream << "}"; // end properties
diff --git a/io/TextWriter.hpp b/io/TextWriter.hpp
index 10b2390d9b..4fbec059b2 100644
--- a/io/TextWriter.hpp
+++ b/io/TextWriter.hpp
@@ -43,6 +43,12 @@ typedef std::shared_ptr<std::ostream> FileStreamPtr;
 
 class PDAL_DLL TextWriter : public Writer
 {
+    struct DimSpec
+    {
+        Dimension::Id id;
+        size_t precision;
+    };
+
 public:
     TextWriter()
     {}
@@ -63,6 +69,8 @@ class PDAL_DLL TextWriter : public Writer
 
     void writeGeoJSONBuffer(const PointViewPtr view);
     void writeCSVBuffer(const PointViewPtr view);
+    DimSpec extractDim(std::string dim, PointTableRef table);
+    bool findDim(Dimension::Id id, DimSpec& ds);
 
     std::string m_filename;
     std::string m_outputType;
@@ -77,7 +85,7 @@ class PDAL_DLL TextWriter : public Writer
     int m_precision;
 
     FileStreamPtr m_stream;
-    Dimension::IdList m_dims;
+    std::vector<DimSpec> m_dims;
 
     TextWriter& operator=(const TextWriter&); // not implemented
     TextWriter(const TextWriter&); // not implemented
diff --git a/test/unit/io/TextWriterTest.cpp b/test/unit/io/TextWriterTest.cpp
index 7d24fc5321..71731526c7 100644
--- a/test/unit/io/TextWriterTest.cpp
+++ b/test/unit/io/TextWriterTest.cpp
@@ -37,6 +37,7 @@
 #include "Support.hpp"
 
 #include <pdal/util/FileUtils.hpp>
+#include <io/BufferReader.hpp>
 #include <io/TextReader.hpp>
 #include <io/TextWriter.hpp>
 
@@ -104,3 +105,51 @@ TEST(TextWriterTest, t2)
 
     EXPECT_EQ(Support::compare_text_files(infile, outfile), true);
 }
+
+TEST(TextWriterTest, precision)
+{
+    using namespace Dimension;
+
+    PointTable table;
+    table.layout()->registerDims( { Id::X, Id::Y, Id::Z, Id::Intensity } );
+
+    PointViewPtr view(new PointView(table));
+    view->setField(Id::X, 0, 1);
+    view->setField(Id::Y, 0, 1);
+    view->setField(Id::Z, 0, 1);
+    view->setField(Id::Intensity, 0, 1);
+
+    view->setField(Id::X, 1, 2.2222222222);
+    view->setField(Id::Y, 1, 2.2222222222);
+    view->setField(Id::Z, 1, 2.2222222222);
+    view->setField(Id::Intensity, 1, 2.22222222);
+
+    view->setField(Id::X, 2, 3.33);
+    view->setField(Id::Y, 2, 3.33);
+    view->setField(Id::Z, 2, 3.33);
+    view->setField(Id::Intensity, 2, 3.33);
+
+    BufferReader r;
+    r.addView(view);
+
+    std::string outfile(Support::temppath("precision.txt"));
+
+    TextWriter w;
+
+    Options o;
+    o.add("precision", 5);
+    o.add("order", "X:0,Y:0,Z:0,Intensity:0");
+    o.add("filename", outfile);
+
+    w.setInput(r);
+    w.setOptions(o);
+
+    w.prepare(table);
+    w.execute(table);
+
+    std::string out = FileUtils::readFileIntoString(outfile);
+    EXPECT_NE(out.find("1,1,1,1"), std::string::npos);
+    EXPECT_NE(out.find("2,2,2,2"), std::string::npos);
+    EXPECT_NE(out.find("3,3,3,3"), std::string::npos);
+}
+