Re: [eigen] Undesired formatting behaviour

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

On Tue, Feb 24, 2009 at 7:24 PM, Johan Pauwels
<johan.pauwels@xxxxxxxxxxxxx> wrote:
> Hi all,
> Replacing
> int width = 0 by int width = s.width(); s.width(0); on line 129 of
> Eigen/src/Core/io.h gave me the result I intended. As far as I can see, this
> patch is compatible with the concept of IOFormat (of which I learned later
> on), since the default width of 0 has no effect.

I agree with that change.

> A similar, but more debatable case is that of ostream.precision(). If you
> specify the precision like cout << setprecision(3) << matrix; it gets
> ignored. The difference with the width case is that I cannot think of a
> straightforward patch that would add this functionality without changing the
> behaviour of IOFormat, since there is no "neutral" value for precision such
> as width(0) is. I guess the thing to do would be to initialise the default
> matPrefix to a to be defined special symbol that tells to take the current
> precision of the ostream when writing to that stream.

I understand your concerns and I think that's generally a good idea to
add default parameter values which would mean "use the respective
setting of the ostream object".

> What I consider an inconsistency is that you use the IOFormat class at all
> to define the precision, since you still are dependent on the floatfield
> flag of the stream to use the precision you define in IOFormat. I mean, you
> still need to write cout << fixed << matrix; before the precision gets used
> with floats for example. Either you define all the formatting behaviour in
> the IOFormat class and inhibit the effects of the ostream flags or you read
> all ostream flags and act accordingly (where I'd prefer the latter). The
> AlignCols flag has added value compared to a simple width(x), so to put that
> into IOFormat makes sense to me in both cases.

the main goal of the current design was to be able to define a format
object once and use it many times. This is why all options are
encapsulated in IOFormat.

> Either way, a trivial feature
> request is that I'd like setters for the variables in IO Format such that you
> can initialise with the default IOFormat() format; and just change the one
> variable you want with format.setRowPrefix = "pre"; for example.

no problem to add such setters, a patch would be very much appreciated ;)

> A last, completely unrelated thing is that I can find how to take the sum of
> a matrix, but not the product of the matrix. It could be that I'm looking
> over it, but if not, it's another feature request.

in svn trunk we have the prod() function which does exactly that.

> I hope this comes over as some constructive criticism as I intend it to be,
> because overall I think it is a terribly well done, feature-complete and
> elegant library.

constructive remarks are always very welcomed :)

gael.