[gnuastro-commits] master 0eb29152: Library (txt.h): Not printing + infront of positive numbers

[Mohammad Akhlaghi]

branch: master
commit 0eb29152845ba77ea9fef1b5e77873e34fdd298c
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (txt.h): Not printing + infront of positive numbers
    
    Until now, when printing any sort of number as plain text, Gnuastro's
    'txt.c' library would place a '+' infront of positive numbers. This was
    done so in multi-row values the positive and negative numbers don't appear
    as a zig-zag when looking at the digits, like below:
    
       Bad         Good
       ---         -----
       1.23        +1.23
       -4.56       -4.56
       7.99        +7.99
       -1.89       -1.89
    
    After this change was implemented Zohreh Ghaffari discovered that programs
    like TOPCAT have problems with the '+' and Sepideh Eskandarlou pointed out
    that the '+' is causing many complications in scripts when positive
    integers were used as filenames. In both cases, they were forced to remove
    the '+' with a 'sed' command before using the number.
    
    With this commit, to have the best of both worlds (a clear visual reading
    of the numbers with the first digits placed under each other without the
    extra '+'), we now put a ' ' instead of the '+'. So the output above looks
    like this:
    
       Bad         Good
       ---         -----
       1.23         1.23
       -4.56       -4.56
       7.99         7.99
       -1.89       -1.89
    
    Ultimately this was done with a correction in the 'printf' call to define
    the formats. However, it also required the following changes:
    
     - During tests suggested by Sepideh Eskandrlou, we discovered that
       'table_bring_to_top' was not setting the 'array' pointer of an empty
       column to NULL!
    
     - During the tests we noticed that the two 'table_sort' and
       'table_select_by_position' functions of 'bin/table/table.c' were only
       checking one of the ways to have an empty table (either size of zero or
       array of NULL). But to be generic it is good to check both.
    
     - Since we rely on many 'printf' features that may not portable in many
       non-GNU operating systems, the Gnulib 'stdio' is now also bootstrapped
       with Gnuastro.
    
     - Defining the new 'gal_statistics_has_negative' function to identify if
       the column has a negative value (and select the format above).
    
     - The 'tests/during-dev.sh' script would not rebuild the programs before
       testing the installed scripts. This would make it hard to test the
       program causing a crash within the installed scripts. From now on, all
       executable programs in the build directory are deleted and re-built
       while testing scripts.
    
    This issue was raised by Zohreh Ghaffari and Sepideh Eskandarlou.
---
 NEWS                      |  1 +
 bin/table/table.c         | 10 ++++---
 bootstrap.conf            |  1 +
 doc/gnuastro.texi         |  5 ++++
 lib/blank.c               |  4 +--
 lib/gnuastro/statistics.h |  3 ++-
 lib/statistics.c          | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/txt.c                 | 33 ++++++++++++++++-------
 tests/during-dev.sh       | 10 +++++++
 9 files changed, 120 insertions(+), 16 deletions(-)

diff --git a/NEWS b/NEWS
index 16dc17fc..401f7bad 100644
--- a/NEWS
+++ b/NEWS
@@ -153,6 +153,7 @@ See the end of the file for license conditions.
      datasets using an identification string (either counter or name).
    - gal_permutation_apply_onlydim0: When we have a 2D input, apply
      permutation for all the elements of each row (along dimension-0 in C).
+   - gal_statistics_has_negative: see if input has a negative value.
    - gal_table_col_vector_extract: extract the given elements of a vector
      column into separate columns.
    - gal_table_cols_to_vector: merge multiple columns into a vector column.
diff --git a/bin/table/table.c b/bin/table/table.c
index f9a70a99..ff1ece15 100644
--- a/bin/table/table.c
+++ b/bin/table/table.c
@@ -156,8 +156,10 @@ table_bring_to_top(gal_data_t *table, gal_data_t *rowids)
          pointers. */
       col->dsize[0] = rowids->size;
       col->size = col->dsize[0] * n;
-    }
 
+      /* If there is no elements, free 'array' and set it to NULL. */
+      if(col->size==0 && col->array) { free(col->array); col->array=NULL; }
+    }
 }
 
 
@@ -566,7 +568,8 @@ table_sort(struct tableparams *p)
   int (*qsortfn)(const void *, const void *)=NULL;
 
   /* In case there are no columns to sort, skip this function. */
-  if(p->table->size==0) return;
+  if(p->table->size==0 || p->table->array==NULL || p->table->dsize==NULL)
+    return;
 
   /* Allocate the permutation array and fill it. Note that we need 'dsize0'
      because the first column may be a vector column (which is 2D). */
@@ -707,7 +710,8 @@ table_select_by_position(struct tableparams *p)
   double *darr = p->rowrange ? p->rowrange->array : NULL;
 
   /* If the table is already empty, then don't bother continuing. */
-  if(p->table->array==NULL) return;
+  if(p->table->size==0 || p->table->array==NULL || p->table->dsize==NULL)
+    return;
 
   /* If the head or tail values are given and are larger than the number of
      rows, just set them to the number of rows (print the all the final
diff --git a/bootstrap.conf b/bootstrap.conf
index 8a040f58..bd7914fc 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -202,6 +202,7 @@ gnulib_modules="
     regex
     error
     nproc
+    stdio
     select
     stdint
     strtod
diff --git a/doc/gnuastro.texi b/doc/gnuastro.texi
index e359e3a3..157cfd43 100644
--- a/doc/gnuastro.texi
+++ b/doc/gnuastro.texi
@@ -37297,6 +37297,11 @@ Note that the input's number of dimensions is 
irrelevant for this function.
 If @code{inplace} is not zero, then the unique values will over-write the 
allocated space of the input, otherwise a new space will be allocated and the 
input will not be touched.
 @end deftypefun
 
+@deftypefun int gal_statistics_has_negative (gal_data_t @code{*input})
+Return @code{1} if the input dataset contains a negative number and @code{0} 
otherwise.
+If the dataset doesn't have a numeric type (as in a string), this function 
will abort with, saying that it does not recognize the file type.
+@end deftypefun
+
 @deftypefun {gal_data_t *} gal_statistics_mode (gal_data_t @code{*input}, 
float @code{mirrordist}, int @code{inplace})
 Return a four-element (@code{double} or @code{float64}) dataset that
 contains the mode of the @code{input} distribution. This function
diff --git a/lib/blank.c b/lib/blank.c
index ee13507d..411eb767 100644
--- a/lib/blank.c
+++ b/lib/blank.c
@@ -463,8 +463,8 @@ gal_blank_present(gal_data_t *input, int updateflag)
     /* String. */
     case GAL_TYPE_STRING:
       if(input!=block)
-        error(EXIT_FAILURE, 0, "%s: tile mode is currently not supported for "
-              "strings", __func__);
+        error(EXIT_FAILURE, 0, "%s: tile mode is currently not "
+              "supported for strings", __func__);
       strf = (str=input->array) + input->size;
       do
         if(*str==NULL || !strcmp(*str,GAL_BLANK_STRING)) return 1;
diff --git a/lib/gnuastro/statistics.h b/lib/gnuastro/statistics.h
index 6fafd836..e9fddf33 100644
--- a/lib/gnuastro/statistics.h
+++ b/lib/gnuastro/statistics.h
@@ -114,7 +114,8 @@ gal_statistics_quantile_function(gal_data_t *input, 
gal_data_t *value,
 gal_data_t *
 gal_statistics_unique(gal_data_t *input, int inplace);
 
-
+int
+gal_statistics_has_negative(gal_data_t *data);
 
 
 
diff --git a/lib/statistics.c b/lib/statistics.c
index bd7da5b3..4990c70f 100644
--- a/lib/statistics.c
+++ b/lib/statistics.c
@@ -691,6 +691,75 @@ gal_statistics_unique(gal_data_t *input, int inplace)
 
 
 
+#define HAS_NEGATIVE(IT) {                                              \
+    IT b, *a=input->array, *af=a+input->size, *start;                   \
+    gal_blank_write(&b, input->type);                                   \
+                                                                        \
+    /* If this is a tile, not a full block. */                          \
+    if(input!=block)                                                    \
+      start=gal_tile_start_end_ind_inclusive(input, block, start_end_inc); \
+                                                                        \
+    /* Go over all the elements. */                                     \
+    while( start_end_inc[0] + increment <= start_end_inc[1] )           \
+      {                                                                 \
+        /* Necessary when we are on a tile. */                          \
+        if(input!=block)                                                \
+          af = ( a = start + increment ) + input->dsize[input->ndim-1]; \
+                                                                        \
+        /* Check for blank values (only for integers: b==b) */          \
+        if(b==b) do if(*a!=b  && *a<0) { hasneg=1; break; } while(++a<af); \
+        else     do if(*a==*a && *a<0) { hasneg=1; break; } while(++a<af); \
+                                                                        \
+        /* Necessary when we are on a tile. */                          \
+        if(input!=block)                                                \
+          increment += gal_tile_block_increment(block, input->dsize,    \
+                                                num_increment++, NULL); \
+        else break;                                                     \
+      }                                                                 \
+  }
+
+int
+gal_statistics_has_negative(gal_data_t *input)
+{
+  int hasneg=0;
+  size_t increment=0, num_increment=1;
+  gal_data_t *block=gal_tile_block(input);
+  size_t start_end_inc[2]={0,block->size-1}; /* -1: this is INCLUSIVE. */
+
+  /* An empty dataset doesn't have any negative values! */
+  if(input->size==0) return 0;
+
+  /* The operation depends on the type of the input. */
+  switch(input->type)
+    {
+    /* Unsigned integer types are always positive */
+    case GAL_TYPE_UINT8:
+    case GAL_TYPE_UINT16:
+    case GAL_TYPE_UINT32:
+    case GAL_TYPE_UINT64:
+      hasneg=0; break;
+
+    /* Types that can have negative values. */
+    case GAL_TYPE_INT8:     HAS_NEGATIVE(int8_t);  break;
+    case GAL_TYPE_INT16:    HAS_NEGATIVE(int16_t); break;
+    case GAL_TYPE_INT32:    HAS_NEGATIVE(int32_t); break;
+    case GAL_TYPE_INT64:    HAS_NEGATIVE(int64_t); break;
+    case GAL_TYPE_FLOAT32:  HAS_NEGATIVE(float);   break;
+    case GAL_TYPE_FLOAT64:  HAS_NEGATIVE(double);  break;
+
+    /* Non-numeric types. */
+    default:
+      error(EXIT_FAILURE, 0, "%s: type code '%d' not recognized",
+            __func__, input->type);
+    }
+
+  /* Return the result. */
+  return hasneg;
+}
+
+
+
+
 
 
 
diff --git a/lib/txt.c b/lib/txt.c
index c06aa919..3cd693c9 100644
--- a/lib/txt.c
+++ b/lib/txt.c
@@ -37,6 +37,7 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 #include <gnuastro/blank.h>
 #include <gnuastro/table.h>
 #include <gnuastro/pointer.h>
+#include <gnuastro/statistics.h>
 
 #include <gnuastro-internal/checkset.h>
 #include <gnuastro-internal/tableintern.h>
@@ -1649,14 +1650,21 @@ static void
 txt_fmts_for_printf_norm(gal_data_t *data, char *fmta, char *lng,
                          char *fmt, int leftadjust)
 {
+  /* Strings should be treated like  */
+  int hasneg = ( data->type==GAL_TYPE_STRING
+                 ? 0
+                 : gal_statistics_has_negative(data) );
+
   /* The space in the end of 'fmts[i*FMTS_COLS]' is to ensure that the
      columns don't merge, even if the printed string is larger than the
      expected width. */
   if(data->disp_precision == GAL_BLANK_INT)
-    sprintf(fmta, "%%+%s%d%s%s ", leftadjust ? "-" : "",
+    sprintf(fmta, hasneg ? "%% %s%d%s%s " : "%%%s%d%s%s ",
+            leftadjust ? "-" : "",
             data->disp_width, lng, fmt);
   else
-    sprintf(fmta, "%%+%s%d.%d%s%s ", leftadjust ? "-" : "",
+    sprintf(fmta, hasneg ? "%% %s%d.%d%s%s " : "%%%s%d.%d%s%s ",
+            leftadjust ? "-" : "",
             data->disp_width, data->disp_precision, lng, fmt);
 }
 
@@ -1665,13 +1673,20 @@ txt_fmts_for_printf_norm(gal_data_t *data, char *fmta, 
char *lng,
 
 
 static void
-txt_fmts_for_printf_last(int disp_precision, char *fmta, char *lng,
+txt_fmts_for_printf_last(gal_data_t *data, char *fmta, char *lng,
                          char *fmt)
 {
-  if(disp_precision == GAL_BLANK_INT)
-    sprintf(fmta, "%%+%s%s", lng, fmt);
+  /* Strings should be treated like  */
+  int hasneg = ( data->type==GAL_TYPE_STRING
+                 ? 0
+                 : gal_statistics_has_negative(data) );
+
+  /* See 'txt_fmts_for_printf_norm' */
+  if(data->disp_precision == GAL_BLANK_INT)
+    sprintf(fmta, hasneg ? "%% %s%s" : "%%%s%s", lng, fmt);
   else
-    sprintf(fmta, "%%+.%d%s%s", disp_precision, lng, fmt);
+    sprintf(fmta, hasneg ? "%% .%d%s%s" : "%%.%d%s%s",
+            data->disp_precision, lng, fmt);
 }
 
 
@@ -1754,13 +1769,11 @@ txt_fmts_for_printf(gal_data_t *datall, int leftadjust, 
int tab0_img1)
             {
               txt_fmts_for_printf_norm(data, fmts[i*FMTS_COLS], lng, fmt,
                                        leftadjust);
-              txt_fmts_for_printf_last(data->disp_precision,
-                                       fmts[i*FMTS_COLS+3], lng, fmt);
+              txt_fmts_for_printf_last(data, fmts[i*FMTS_COLS+3], lng, fmt);
             }
           else /* Last column is not a vector. */
             {
-              txt_fmts_for_printf_last(data->disp_precision, fmts[i*FMTS_COLS],
-                                       lng, fmt);
+              txt_fmts_for_printf_last(data, fmts[i*FMTS_COLS], lng, fmt);
               fmts[i*FMTS_COLS+3][0]='\0';
             }
         }
diff --git a/tests/during-dev.sh b/tests/during-dev.sh
index 0ddcd230..7678d7ae 100755
--- a/tests/during-dev.sh
+++ b/tests/during-dev.sh
@@ -82,6 +82,11 @@ outdir=
 # script, and once for the utility. In such cases it might be easier to
 # just add the argument/option to the final script that runs the utility
 # rather than these variables.
+#
+# TEST ASTSCRIPTs: if the problem is in the compiled programs used within
+# the script, you have to add a line under the line below
+#    'if [ -f "$utility" ]; then rm "$utility"; fi'
+# that will delete that particualr program.
 utilname=
 arguments=
 options=
@@ -124,6 +129,11 @@ fi
 longprefix="${utilname:0:6}"
 if [ x"$longprefix" = x"script" ]; then
     execdir="script"
+
+    # We need to delete all the compiled programs so they are recompiled
+    # (for usage in the script, if this slows down your tests, comment it
+    # and only delete the particular program that is causing problems).
+    find "$builddir/bin" -type f -executable -exec rm "{}" \;
 else
     execdir="$utilname"
 fi