Add StringFormatted, PrintFormatted, PrintToFormatted #2200

ChrisJefferson · 2018-02-23T00:16:43Z

This PR adds a simple method of formatting strings, strongly inspired by Python's "format" function. It is probably easiest to demonstrate by example:

# {} goes through a list in turn
gap> Format("I have {} cats and {} dogs", [3,2]);
"I have 3 cats and 2 dogs"
# Or use {<number>} to take a specific element from a list
gap> Format("I have {2} cats and {1} dogs", [3,2]);
"I have 2 cats and 3 dogs"
# Or use {<string>} to pick elements of a record
gap> Format("I have {apple} cats and {pear} dogs", rec(apple:=3, pear:=2);
"I have 3 cats and 2 dogs"
# Use {{ and }} to actually print a { or a }
gap> Format("I want an {{x and a {x}", rec(x := 3));
"I want an {x and a 3"

There is also a FormatTo variant, which prints to a stream.

I've been using this for a while, as I find building these types of simple format strings is quite painful in GAP. There is some other stuff from Python (padding, formatting of floating points), I haven't implemented yet, and might, but I thought first I'd check if this function seemed generally interesting to people.

markuspf

One of the problems I see coming is that

markuspf · 2018-02-23T09:43:04Z

lib/string.gi


+InstallGlobalFunction(FormatTo, function(stream, s, args)
+    local pos, len, outstr, nextbrace, endbrace, id, argcounter;


Is there a (good) reason why you don't use args...?

There is the option to pass a record, but that can be handled even in the presence of args...

Good question. I started off with a list, but there isn't really a good reason for that. I'll switch to args...

markuspf · 2018-02-23T09:46:14Z

lib/string.gi


+InstallGlobalFunction(FormatTo, function(stream, s, args)


markuspf · 2018-02-23T09:51:55Z

lib/string.gi

+                if Int(id) < 1 or Int(id) > Length(args) then
+                    ErrorNoReturn("out of bounds in Format -- asked for {",Int(id),"} when there are only ",Length(args), " arguments");
+                fi;
+                AppendTo(stream, String(args[Int(id)]));


String is problematic because not many objects support String and just return <object>, also one could argue that String builds up a buffer, whereas PrintTo or AppendTo let the object's implementor decide what to do.

why not just args[Int(id)]?

There is of course the question whether you want to support String, Display and View as format strings...

See also #1829

I'm happy to pick which we want (string, display or view), but if you just pass in strings then of course they'll be printeed fine (I could check IsString to avoid the extra call), but it seems a pain to make people call String if they just want to pass in say a permutation.

I'm not sure about DisplayString (they generally don't compose very well) but one way or another it could be useful to be able to include either the ViewString or the String of an object, depending on what you want the formatted string for. Could be an option encoded somehow in the format-string, or a second function FormatView (or some better name).

For consistency with names elsewhere the function should ideally be a noun describing the result, rather than a verb which suggests it modifies its argument. We have FormattedString but it's marked obsolete, so could possibly be resused.

Re String/Display/View: In a printf style function, I'd handle that with different placeholders in the format string, say: Format("%{display} vs. %{view} vs. %{string}", myObj, myObj, myObj); to get a comparison of the three strings.

Looking at https://pyformat.info, I guess the "pythonic" way would be to use a "value conversion" suffix, like Format("{0!d} vs. {0!v}. vs {0!s}", myObj). If we default to using String, then there could also be something to force "raw" output, i.e. no conversion. Or conversely, we could default to not using String (as @markuspf suggested), but still allow people to enable it via a value conversion suffix.

markuspf · 2018-02-23T09:52:27Z

I think we want this, but it might need some additional thought...

frankluebeck · 2018-03-02T08:27:08Z

I have never missed such a function but can see that people find this useful. I'm only a bit concerned about its proposed name. I had a vague plan to propose "Format" as a name for some improved version of String which gets a second argument like '"Text", '*HTML", "LaTeX" (and maybe further arguments). Some version of this exists in Jean Michel's version of GAP3.

Would it be ok for you to change the name? Say, ComplementedString?

ChrisJefferson · 2018-03-02T11:39:03Z

I am happy to change the name. I started with Format just because that is what Python uses, and I based my inspiration on that. I will think about other possible names.

PrintFormatted and PrintFormattedTo (or even.. Printf and PrintfTo, although it's not the same style as C printf?)

fingolfin · 2018-03-03T23:00:03Z

While I don't mind PrintFormatted, I find the argument @frankluebeck raises as to why Format is problematic somewhat strange -- essentially it seems a case of "calling dibs" on a name?!? I.e. you seem to say that there is a possibility you might (or not) one day submit code which may perhaps want to use the name Format for another function?

That's not an argument I'd expect. Perhaps what is meant here is that "Jean Michel's version of GAP3" has a function Format, and the desire here is to have a similar function with the same name in GAP 4, which I'd find somewhat more compelling. Albeit not very much.

Moreover, the function in this PR seems to me (quite subjectively, of course) more useful to a wider audience than a function which produces HTML or LaTeX output (I can't really judge the latter, though, as I don't really know what it's supposed to be). So I'd rather have this here under the name Format than some function to produce HTML/LaTeX whatever which I likely would never use.

I am not so happy with the name PrintFormatted, because it suggests to me that output will be sent to stdout, not into a string. That is something I find a bit weird in the current PR, too, though; i.e. that Formatted returns a string, while FormattedTo prints into a stream. I'd find FormattedString (new name for Format), PrintFormatted (does not exist in this PR right now) and PrintFormattedTo (new name for FormatTo) more logical then. (This also matches an earlier suggestion by @stevelinton).

fingolfin · 2018-03-04T11:33:28Z

Of course, as I just realize, those names kind of clash with GAPDoc's PrintFormattedString sigh

frankluebeck · 2018-03-06T08:43:00Z

@fingolfin: I don't understand what you want to say with your lengthy comment. I certainly didn't give a strong argument and this is not the place to discuss another function.

But I think there is a point that Format might be a compact name for more general functionality and should not be used up for the specialized string substitution function proposed here.

Of course, if people disagree we will find another name for future functions.

In the context of PrintFormattedString the Formatted means that the line breaks should not be changed while the string is printed.

ChrisJefferson · 2018-03-25T11:48:43Z

I've now gone for StringFormatted, PrintFormatted and PrintToFormatted (which are, hopefully obviously, designed to mirror String, Print and PrintTo, with a formatting option).

codecov · 2018-03-25T14:47:27Z

Codecov Report

Merging #2200 into master will increase coverage by 0.01%.
The diff coverage is 96.73%.

@@            Coverage Diff             @@
##           master    #2200      +/-   ##
==========================================
+ Coverage   72.65%   72.66%   +0.01%     
==========================================
  Files         478      478              
  Lines      246633   246726      +93     
==========================================
+ Hits       179187   179284      +97     
+ Misses      67446    67442       -4

Impacted Files	Coverage Δ
lib/string.gi	`47.87% <96.73%> (+6.56%)`	⬆️
src/hpc/traverse.c	`95.75% <0%> (-0.59%)`	⬇️
src/hpc/threadapi.c	`37.41% <0%> (ø)`	⬆️
hpcgap/lib/package.gi	`68.48% <0%> (+0.04%)`	⬆️
lib/package.gi	`69.89% <0%> (+0.04%)`	⬆️
hpcgap/lib/hpc/stdtasks.g	`38.87% <0%> (+0.25%)`	⬆️
lib/record.gi	`69.82% <0%> (+0.86%)`	⬆️
hpcgap/lib/hpc/queue.g	`69.59% <0%> (+3.19%)`	⬆️

fingolfin · 2018-03-27T13:56:10Z

lib/string.gd

+##  <Func Name="PrintToFormatted" Arg='stream, string, data...'/>
+##
+##  <Description>
+##  These functions format an input string.


Do they, though? Don't they rather take a format specification, and a bunch of input data, and use the format string to format the data, and turn that into a string (resp. print it to a stream)?

Looking at Python's str.format:

Perform a string formatting operation. The string on which this method is called can contain literal text or replacement fields delimited by braces {}. Each replacement field contains either the numeric index of a positional argument, or the name of a keyword argument. Returns a copy of the string where each replacement field is replaced with the string value of the corresponding argument. [... example ...] See Format String Syntax for a description of the various formatting options that can be specified in format strings.

Perhaps this can serve as some inspiration?

fingolfin · 2018-03-27T13:56:35Z

lib/string.gd

+##  <Ref Func="PrintToFormatted"/> appends the formatted string to <A>stream</A>,
+##  which can be either an output stream or a filename.
+##  <P/>
+##  <A>data</A> is a list of values to printed, using the formatting rules below:


"to be printed"

Also, I think this can be confusing: the user might think that the second (resp third, in case of PrintToFormatted) argument should be a list, containing the values. Perhaps like this:

All arguments after <A>string</A> form the list <A>data</A> of values to be printed, ...

fingolfin · 2018-03-27T13:57:53Z

lib/string.gd

+##  <A>string</A> is treated as a normal string, except for occurrences
+##  of <C>{</C> and <C>}</C>, which follow special rules, as follows:
+##
+##  The contents of <C>{ }</C> form a small language. The format is <C>{id!format}</C>,


"form a small language" -> is that helpful the reader? What helpful information is it meant to convey? I can't think of any right now, to me it sounds mostly scary. But of course that may be highly subjective!

fingolfin · 2018-03-27T13:58:21Z

lib/string.gd

+##  </Item>
+##  </List>
+##
+##  The <C>format</C> decides how the variable is printed. <C>format</C> must be one


"decides" -> "determines"

fingolfin · 2018-03-27T14:04:28Z

lib/string.gd

+##  <Mark>An integer <C>i</C></Mark> <Item>
+##    Take the <C>i</C>th element of <A>data</A>.
+##  </Item>
+##  <Mark>No variable</Mark> <Item>


What does "No variable" mean here? Perhaps "Not given" or, better, "No id given" ?

fingolfin · 2018-03-27T14:05:43Z

lib/string.gd

+##  <Mark>No variable</Mark> <Item>
+##    Take the <C>j</C>th element of <A>data</A>, where <C>j</C> is the
+##    number of occurrences of <C>{}</C> in the string, including this one,
+##    up to this point.


Doesn't this also count {!format}?

fingolfin · 2018-03-27T14:06:52Z

lib/string.gd

+##  </Item>
+##  <Mark>A string <C>str</C></Mark> <Item>
+##    Take the <C>str</C> member of the first element of <A>data</A>, which 
+##    must be a record.


I like to have "definitions" before using them, in this case: saying that the first entry must be a record before makign use of it (by taking about its "str member"). How about this:

If this is used, then the first element of data must be a record r; in this case, the value r.(str) is taken.

ChrisJefferson · 2018-03-28T18:19:32Z

This is now (I think) fixed up.

fingolfin · 2018-03-28T20:25:40Z

lib/string.gd

+##  Each replacement field contains a numeric or positional argument,
+##  describing the element of <A>data</A> to replace the braces with.
+##
+##  There are three Formatting functions, which differ only in how they


Formatting -> formatting ?

fingolfin · 2018-03-28T20:27:53Z

lib/string.gd

+##  inserted into <A>string</A>, using the following formatting rules:
+##  <P/>
+##  <A>string</A> is treated as a normal string, except for occurrences
+##  of <C>{</C> and <C>}</C>, which follow special rules, as follows:


Actually, re-reading this now, it occurred to me that it's unclear how one can insert braces into the format string that are not interpolated away? In Python this can be done via double braces {{ and }}.

UPDATE: after looking at the example below, and studying the code, it seem you support this, too! Great! But it's not documented, is it? Should be.... ;-)

fingolfin · 2018-03-28T20:30:55Z

lib/string.gd

+##
+##  <Description>
+##  These functions perform a string formatting operation.
+##  They accept an input string, which can contain fields


Perhaps "format string" instead of "input string"?

fingolfin · 2018-03-28T20:33:50Z

lib/string.gd

+##  <Description>
+##  These functions perform a string formatting operation.
+##  They accept an input string, which can contain fields
+##  to be replaced, which are denoted with {}.


This sounds as if only {} is allowed.

How about this (and then you can remove or shorten the mention of data later on):

They accept a format string, followed by an arbitrary number of additional arguments forming a list data.
The format string can contain replacement fields delimited by braces {}.

fingolfin · 2018-03-28T20:39:34Z

lib/string.gd

+##  </Item>
+##  <Mark>No id given</Mark> <Item>
+##    Take the <C>j</C>th element of <A>data</A>, where <C>j</C> is the
+##    number of occurrences of <C>{}</C> in the string with no id given,


Maybe

... the number of previous/prior/preceding replacement fields without an id ...

BTW, this deviates from what Python does; there, either no replacement field has an id, or all must have one.

Oh, I mis-interpreted what python did. I think I'll switch to this, as it seems simpler.

fingolfin · 2018-03-28T21:03:08Z

lib/string.gi

+    pos := 0;
+
+    if not (IsOutputStream(stream) or IsString(stream)) or not IsString(s) then
+        ErrorNoReturn("Usage: PrintToFormatted(<stream>, <string>, <args>...)");


Replace args by data to match documentation

fingolfin · 2018-03-28T21:03:44Z

lib/string.gi


+InstallGlobalFunction(PrintToFormatted, function(stream, s, args...)


Replace args by data to match documentation? (Or vice versa, adjust the documentation)

fingolfin · 2018-03-28T21:04:39Z

lib/string.gi

+
+        if toprint.id = "" then
+            if Length(args) < argcounter then
+                ErrorNoReturn("out of bounds in StringFormatted -- used ",argcounter," {} when there are only ",Length(args), " arguments");


change "{}" in this error message to "replacement fields without id"

fingolfin · 2018-03-28T21:05:48Z

lib/string.gi

+          AppendTo(stream, ViewString(var));
+        elif toprint.type = "d" then
+          AppendTo(stream, DisplayString(var));
+        else ErrorNoReturn("Invalid formatting option: '", toprint.type, "'");


And a third name: after "format" and "type" this is now called "formatting option".

fingolfin · 2018-03-28T21:09:10Z

tst/testinstall/format.tst

+"SymmetricGroup( [ 3 .. 5 ] ) Alt( [ 1, 3 .. 5 ] ) <object>\n"
+gap> StringFormatted("{a!s} {b!v} {c!d}", rec(a := r1, b := r2, c := r3));
+"SymmetricGroup( [ 3 .. 5 ] ) Alt( [ 1, 3 .. 5 ] ) <object>\n"
+gap> StringFormatted("{a!x}", rec(a := r1));


I find it difficult to read this test file, and to see what's there -- so if I wanted to add more tests, I'd struggle to decide where to do so.

Perhaps consider structuring this test file a bit by inserting some empty lines to separate tests into "logical" groups? (Well, empty lines followed by a comment, to suppress whitespace diffs in the test; like this:

gap> StringFormatted("{1}", rec()); "rec( )" # test format options gap> r1 := SymmetricGroup([3..5]);; gap> r2 := AlternatingGroup([1,3,5]);;

fingolfin · 2018-03-29T07:40:13Z

lib/string.gd

+##
+##  A single brace can be outputted by doubling, so <C>{{</C> in the format string
+##  produces <C>{</C> and <C>}}</C> produces <C>}</C>.
+##  </P>


syntax error, change </P> to <P/>

fingolfin

Thanks, I am pretty much happy now. One XML syntax error, and two minor phrasing suggestions left.

fingolfin · 2018-03-29T07:41:09Z

lib/string.gd

+##  <Description>
+##  These functions perform a string formatting operation.
+##  They accept a format string, which can contain replacement fields
+##  which are delimated by braces {}.


delimated -> delimited

fingolfin · 2018-03-29T07:43:32Z

lib/string.gd

+##  which can be either an output stream or a filename.
+##  <P/>
+##  The arguments after <A>string</A> form a list <A>data</A> of the values to be
+##  inserted into <A>string</A>, using the following formatting rules:


Nitpick: the values are not inserted into string (which indeed can be immutable). Perhaps

... form a list data of values used to substitute the replacement fields in string, according to the following formatting rules

This PR adds a simple method of formatting strings, strongly inspired by Python's "format" function. These 3 functions mirror Sting, Print and PrintTo.

fingolfin

No complaints from me this time around :-)

markuspf reviewed Feb 23, 2018

View reviewed changes

ChrisJefferson force-pushed the format branch 2 times, most recently from c4ae523 to 60eb80b Compare March 1, 2018 17:52

ChrisJefferson force-pushed the format branch from 60eb80b to 0d7e272 Compare March 2, 2018 09:58

ChrisJefferson force-pushed the format branch from 0d7e272 to c1017b1 Compare March 25, 2018 11:47

ChrisJefferson force-pushed the format branch 2 times, most recently from 29175da to e1b7fcb Compare March 25, 2018 12:52

fingolfin reviewed Mar 27, 2018

View reviewed changes

ChrisJefferson force-pushed the format branch 2 times, most recently from abd917f to 0d43705 Compare March 27, 2018 16:19

fingolfin reviewed Mar 28, 2018

View reviewed changes

ChrisJefferson force-pushed the format branch from 0d43705 to 20f30a4 Compare March 28, 2018 22:16

fingolfin reviewed Mar 29, 2018

View reviewed changes

fingolfin approved these changes Mar 29, 2018

View reviewed changes

Add StringFormatted, PrintFormatted, PrintToFormatted.

917e1e3

This PR adds a simple method of formatting strings, strongly inspired by Python's "format" function. These 3 functions mirror Sting, Print and PrintTo.

ChrisJefferson force-pushed the format branch from 20f30a4 to 917e1e3 Compare March 29, 2018 18:11

fingolfin added kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements kind: new feature labels Mar 29, 2018

fingolfin approved these changes Mar 29, 2018

View reviewed changes

fingolfin changed the title ~~Add a simple Format function~~ Add StringFormatted, PrintFormatted, PrintToFormatted Mar 29, 2018

fingolfin merged commit a29a129 into gap-system:master Mar 29, 2018

ChrisJefferson deleted the format branch June 11, 2018 19:02

fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 20, 2018


		InstallGlobalFunction(FormatTo, function(stream, s, args)
		local pos, len, outstr, nextbrace, endbrace, id, argcounter;


		InstallGlobalFunction(PrintToFormatted, function(stream, s, args...)

Add StringFormatted, PrintFormatted, PrintToFormatted #2200

Add StringFormatted, PrintFormatted, PrintToFormatted #2200

Conversation

ChrisJefferson commented Feb 23, 2018 • edited Loading

markuspf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markuspf commented Feb 23, 2018

frankluebeck commented Mar 2, 2018

ChrisJefferson commented Mar 2, 2018 • edited Loading

fingolfin commented Mar 3, 2018

fingolfin commented Mar 4, 2018

frankluebeck commented Mar 6, 2018

ChrisJefferson commented Mar 25, 2018

codecov bot commented Mar 25, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisJefferson commented Mar 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fingolfin left a comment

Choose a reason for hiding this comment

ChrisJefferson commented Feb 23, 2018 •

edited

Loading

ChrisJefferson commented Mar 2, 2018 •

edited

Loading

codecov bot commented Mar 25, 2018 •

edited

Loading