Bug-fixes to float formatting and some tests

jeff5 · jeff5 · commit a846473d354a · 2025-12-29T17:02:01.000Z
We make some improvements to grouping and the default precision. There
are no doubt many other Python 3 advances we should catch up with, but
they can wait.
diff --git a/rt4core/src/main/java/uk/co/farowl/vsj4/core/PyFloat.java b/rt4core/src/main/java/uk/co/farowl/vsj4/core/PyFloat.java
@@ -287,29 +287,28 @@ private static String formatDouble(double value,
     static class Formatter extends FloatFormatter {
 
         /**
-         * If {@code true}, give {@code printf}-style meanings to
+         * If {@code true}, give old-style %-formatting meanings to
          * {@link FormatSpec#type}.
          */
-        final boolean printf;
+        final boolean oldfmt;
 
         /**
          * Prepare a {@link Formatter} in support of
-         * {@code str.__mod__}, that is, traditional
-         * {@code printf}-style formatting.
+         * {@code str.__mod__}, that is, old-style %-formatting.
          *
          * @param spec a parsed format specification.
-         * @param printf f {@code true}, interpret {@code spec}
-         *     {@code printf}-style, otherwise as
+         * @param oldfmt f {@code true}, interpret {@code spec}
+         *     old-style %-formatting, otherwise as
          *     {@link Formatter#Formatter(FormatSpec)
          *     Formatter(FormatSpec)}
          * @throws FormatOverflow if a value is out of range (including
          *     the precision)
          * @throws FormatError if an unsupported format character is
          *     encountered
          */
-        Formatter(FormatSpec spec, boolean printf) throws FormatError {
-            super(validated(spec, printf));
-            this.printf = printf;
+        Formatter(FormatSpec spec, boolean oldfmt) throws FormatError {
+            super(validated(spec, oldfmt));
+            this.oldfmt = oldfmt;
         }
 
         /**
@@ -334,14 +333,14 @@ static class Formatter extends FloatFormatter {
          * @throws FormatError on failure to validate
          */
         private static FormatSpec validated(FormatSpec spec,
-                boolean printf) throws FormatError {
-            String type = TYPE.getName();
+                boolean oldfmt) throws FormatError {
 
             switch (spec.type) {
 
                 case 'n':
-                    if (spec.grouping) {
-                        throw notAllowed("Grouping", type, spec.type);
+                    if (spec.grouper != 0) {
+                        throw notAllowed("Grouping", spec.grouper,
+                                TYPE.getName(), spec.type);
                     }
                     //$FALL-THROUGH$
 
@@ -353,21 +352,22 @@ private static FormatSpec validated(FormatSpec spec,
                 case 'F':
                 case 'G':
                 case '%':
+                    // TODO Alternate forms allowed in Python 3
                     // Check for disallowed parts of the specification
                     if (spec.alternate) {
-                        throw alternateFormNotAllowed(type);
+                        throw alternateFormNotAllowed(TYPE.getName());
                     }
                     break;
 
                 case 'r':
                 case 's':
-                    // Only allow for printf-style formatting
-                    if (printf) { break; }
+                    // Only allow for old-style %-formatting
+                    if (oldfmt) { break; }
                     //$FALL-THROUGH$
 
                 default:
                     // The type code was not recognised
-                    throw unknownFormat(spec.type, type);
+                    throw unknownFormat(spec.type, TYPE.getName());
             }
 
             /*
diff --git a/rt4core/src/main/java/uk/co/farowl/vsj4/stringlib/FloatFormatter.java b/rt4core/src/main/java/uk/co/farowl/vsj4/stringlib/FloatFormatter.java
@@ -10,7 +10,6 @@
 import uk.co.farowl.vsj4.stringlib.InternalFormat.FormatOverflow;
 import uk.co.farowl.vsj4.stringlib.InternalFormat.FormatSpec;
 
-
 /**
  * A class that provides the implementation of floating-point
  * formatting. In a limited way, it acts like a StringBuilder to which
@@ -188,7 +187,8 @@ public FloatFormatter format(double value)
      * here. At the point this is used, we know the {@link #spec} is one
      * of the floating-point types. This entry point allows explicit
      * control of the prefix of positive numbers, overriding defaults
-     * for the format type.
+     * for the format type. (We use this before the imaginary part of a
+     * complex number.)
      *
      * @param value to convert
      * @param positivePrefix to use before positive values (e.g. "+") or
@@ -208,9 +208,9 @@ public FloatFormatter format(double value, String positivePrefix)
          */
         setStart();
 
-        // Precision defaults to 6 (or 12 for none-format)
-        int precision =
-                spec.getPrecision(FormatSpec.specified(spec.type) ? 6 : 12);
+        // Precision defaults to 6 (or 17 for none-format)
+        int precision = spec
+                .getPrecision(FormatSpec.specified(spec.type) ? 6 : 17);
 
         // Guard against excessive result precision
         // XXX Possibly better raised before result is allocated/sized.
@@ -301,7 +301,7 @@ public FloatFormatter format(double value, String positivePrefix)
         if (Character.isUpperCase(spec.type)) { uppercase(); }
 
         // If required to, group the whole-part digits.
-        if (spec.grouping) { groupDigits(3, ','); }
+        if (spec.group > 0) { groupDigits(spec.group, spec.grouper); }
 
         return this;
     }
@@ -535,7 +535,8 @@ private void ensurePointAndTrailingZeros(int n) {
      * {@link BigDecimal} to provide conversion and rounding. These
      * variants are g-format proper, alternate g-format (available for
      * "%#g" formatting), n-format (as g but subsequently
-     * "internationalised"), and none-format (type code FormatSpec.NONE).
+     * "internationalised"), and none-format (type code
+     * FormatSpec.NONE).
      * <p>
      * None-format is the basis of {@code float.__str__}.
      * <p>
@@ -611,7 +612,7 @@ private void format_g(double value, String positivePrefix,
         // Precision 0 behaves as 1
         precision = Math.max(1, precision);
 
-        // Use exponential notation if exponent would be bigger thatn:
+        // Use exponential notation if exponent would be bigger than:
         int expThreshold = precision + expThresholdAdj;
 
         if (signAndSpecialNumber(value, positivePrefix)) {
diff --git a/rt4core/src/main/java/uk/co/farowl/vsj4/stringlib/InternalFormat.java b/rt4core/src/main/java/uk/co/farowl/vsj4/stringlib/InternalFormat.java
@@ -69,7 +69,7 @@ public static abstract class AbstractFormatter
         protected int lenSign;
         /**
          * The length of the whole part (to left of the decimal point or
-         * exponent)
+         * exponent).
          */
         protected int lenWhole;
 
@@ -461,13 +461,21 @@ protected void pad(int leftIndex, int n) {
             // Append the trailing space
             for (int i = 0; i < trailing; i++) { result.append(fill); }
 
-            // Check for special case
-            if (align == '=' && fill == '0' && spec.grouping) {
+            /*
+             * Check for special case where we shall extend the grouping
+             * already applied to the whole part into the left
+             * 0-padding.
+             */
+            if (align == '=' && fill == '0' && spec.group > 0) {
                 /*
-                 * We must extend the grouping separator into the
-                 * padding
+                 * Extend the grouping separator into the padding,
+                 * EXCEPT if there was originally no whole part (occurs
+                 * for inf, nan).
                  */
-                zeroPadAfterSignWithGroupingFixup(3, ',');
+                if (lenWhole > leading) {
+                    zeroPadAfterSignWithGroupingFixup(spec.group,
+                            spec.grouper);
+                }
             }
         }
 
@@ -505,7 +513,7 @@ protected void pad(int leftIndex, int n) {
          * '-<b>0</b>,000,000,001,200,000,000.0000'
          * </pre>
          *
-         * @param groupSize normally 3.
+         * @param groupSize normally 3 or 4.
          * @param comma or some other character to use as a separator.
          */
         protected void zeroPadAfterSignWithGroupingFixup(int groupSize,
@@ -787,11 +795,10 @@ public static class FormatSpec {
         public final boolean alternate;
         /** Width to which to pad the result, or -1 if unspecified. */
         public final int width;
-        /**
-         * Insert the grouping separator (which in Python always
-         * indicates a group-size of 3).
-         */
-        public final boolean grouping;
+        /** Grouping size (or &le;0 for no grouping)). */
+        public final int group;
+        /** The group separator (if {@link #group} is &gt;0). */
+        public final char grouper;
         /** Precision decoded from the format, or -1 if unspecified. */
         public final int precision;
         /** Type key from the format, or U+FFFF if unspecified. */
@@ -827,7 +834,7 @@ public static final boolean specified(char c) {
          *     has been specified).
          */
         public static final boolean specified(int value) {
-            return value >= 0;
+            return value >= 0; // UNSPECIFIED = -1
         }
 
         /**
@@ -845,19 +852,21 @@ public static final boolean specified(int value) {
          * @param alternate true to request alternate formatting mode
          *     ({@code '#'} flag).
          * @param width of field after padding or -1 to default
-         * @param grouping true to request comma-separated groups
+         * @param group if non-zero digit group size (usually 3 or 4)
+         * @param grouper character separating groups
          * @param precision (e.g. decimal places) or -1 to default
          * @param type indicator character
          */
         public FormatSpec(char fill, char align, char sign,
-                boolean alternate, int width, boolean grouping,
+                boolean alternate, int width, int group, char grouper,
                 int precision, char type) {
             this.fill = fill;
             this.align = align;
             this.sign = sign;
             this.alternate = alternate;
             this.width = width;
-            this.grouping = grouping;
+            this.group = group;
+            this.grouper = grouper;
             this.precision = precision;
             this.type = type;
         }
@@ -874,7 +883,7 @@ public String toString() {
             if (specified(sign)) { buf.append(sign); }
             if (alternate) { buf.append('#'); }
             if (specified(width)) { buf.append(width); }
-            if (grouping) { buf.append(','); }
+            if (specified(group)) { buf.append(grouper); }
             if (specified(precision)) {
                 buf.append('.').append(precision);
             }
@@ -908,7 +917,8 @@ public FormatSpec withDefaults(FormatSpec other) {
                     specified(sign) ? sign : other.sign, //
                     alternate || other.alternate, //
                     specified(width) ? width : other.width, //
-                    grouping || other.grouping, //
+                    specified(group) ? group : other.group, //
+                    specified(grouper) ? grouper : other.grouper, //
                     specified(precision) ? precision : other.precision, //
                     specified(type) ? type : other.type //
             );
@@ -918,16 +928,16 @@ public FormatSpec withDefaults(FormatSpec other) {
          * Defaults applicable to most numeric types. Equivalent to "
          * &gt;"
          */
-        public static final FormatSpec NUMERIC = new FormatSpec(' ',
-                '>', FormatSpec.NONE, false, FormatSpec.UNSPECIFIED,
-                false, FormatSpec.UNSPECIFIED, FormatSpec.NONE);
+        public static final FormatSpec NUMERIC =
+                new FormatSpec(' ', '>', NONE, false, UNSPECIFIED,
+                        UNSPECIFIED, NONE, UNSPECIFIED, NONE);
 
         /**
          * Defaults applicable to string types. Equivalent to " &lt;"
          */
-        public static final FormatSpec STRING = new FormatSpec(' ', '<',
-                FormatSpec.NONE, false, FormatSpec.UNSPECIFIED, false,
-                FormatSpec.UNSPECIFIED, FormatSpec.NONE);
+        public static final FormatSpec STRING =
+                new FormatSpec(' ', '<', NONE, false, UNSPECIFIED,
+                        UNSPECIFIED, NONE, UNSPECIFIED, NONE);
 
         /**
          * Constructor offering just precision and type.
@@ -940,7 +950,7 @@ public FormatSpec withDefaults(FormatSpec other) {
          * @param type indicator character
          */
         public FormatSpec(int precision, char type) {
-            this(' ', '>', FormatSpec.NONE, false, UNSPECIFIED, false,
+            this(' ', '>', NONE, false, UNSPECIFIED, UNSPECIFIED, NONE,
                     precision, type);
         }
 
@@ -1009,6 +1019,11 @@ private static class Parser {
             this.ptr = 0;
         }
 
+        @Override
+        public String toString() {
+            return spec.substring(0, ptr) + "|" + spec.substring(ptr);
+        }
+
         /**
          * Parse the specification with which this object was
          * initialised into an {@link FormatSpec}, which is an object
@@ -1029,11 +1044,13 @@ private static class Parser {
          */
         FormatSpec parse() {
 
-            char fill = FormatSpec.NONE, align = FormatSpec.NONE;
-            char sign = FormatSpec.NONE, type = FormatSpec.NONE;
-            boolean alternate = false, grouping = false;
-            int width = FormatSpec.UNSPECIFIED,
-                    precision = FormatSpec.UNSPECIFIED;
+            char fill = FormatSpec.NONE, align = FormatSpec.NONE,
+                    sign = FormatSpec.NONE, type = FormatSpec.NONE,
+                    grouper = FormatSpec.NONE;
+            boolean alternate = false;
+            int width = FormatSpec.UNSPECIFIED;
+            int precision = FormatSpec.UNSPECIFIED;
+            int group = FormatSpec.UNSPECIFIED;
 
             // Scan [[fill]align] ...
             if (isAlign()) {
@@ -1082,8 +1099,8 @@ FormatSpec parse() {
             // Scan [width]
             if (isDigit()) { width = scanInteger(); }
 
-            // Scan [,][.precision][type]
-            grouping = scanPast(',');
+            // Scan [grouper][.precision][type]
+            if (isAt(",_")) { grouper = spec.charAt(ptr++); }
 
             // Scan [.precision]
             if (scanPast('.')) {
@@ -1098,6 +1115,12 @@ FormatSpec parse() {
             // Scan [type]
             if (ptr < spec.length()) { type = spec.charAt(ptr++); }
 
+            // If grouping was specified, work out the group size.
+            if (grouper != FormatSpec.NONE) {
+                // Hex, octal and binary group in 4s. Otherwise 3s.
+                group = "boxX".indexOf(type) < 0 ? 3 : 4;
+            }
+
             // If we haven't reached the end, something is wrong
             if (ptr != spec.length()) {
                 throw new IllegalArgumentException(
@@ -1106,7 +1129,7 @@ FormatSpec parse() {
 
             // Create a specification
             return new FormatSpec(fill, align, sign, alternate, width,
-                    grouping, precision, type);
+                    group, grouper, precision, type);
         }
 
         /**
@@ -1152,7 +1175,6 @@ private int scanInteger() {
             while (isDigit()) { ptr++; }
             return Integer.parseInt(spec.substring(p, ptr));
         }
-
     }
 
     /**
diff --git a/rt4core/src/test/java/uk/co/farowl/vsj4/core/FormatTest.java b/rt4core/src/test/java/uk/co/farowl/vsj4/core/FormatTest.java
