sclang: print floats with at least 1 decimal place

[mossheim]

Fixes #3573

Floats that are equal to an integer are printed with .0

I couldn't find a way to do this without actually scanning the string to
see if '.0' needs to be appended. "%.9f" will print extra significant
digits; and "%#.14g" will do the same. I think possibly switching on
the point where numbers change to scientific notation (1e14) and then
using "%#.14f" would work, but I'm not sure if that's a portable
solution and this seems a bit cleaner.

---

Before:

sc3> 3.0
-> 3

After:

sc3> 3.0
-> 3.0
sc3> 3.0000
-> 3.0
sc3> 3 - 0.0000000000000001
-> 3.0

[aspiers]

Awesome, thanks!

[telephon]

Ah good.

will inf return inf and not inf.0. And also the nan?

We'll be able to get rid of the code of storeOn in Float.sc:

storeOn { |stream|
		var	str;
		if(this == inf) { stream << "inf"; ^this };
		if(this == -inf) { stream << "-inf"; ^this };
		str = super.asString;
		stream << str;
		// if it doesn't already have a . or is 1e-05 then add a .0 to force it to Float
		if(str.find(".").isNil and: { str.find("e").isNil }) {
			stream << ".0";
		}
	}

actually, I just found a bug here:

(0/0).asCompileString // returns nan.0

[mossheim]

@telephon - good catch! I've patched it and added tests, they pass locally.

Do you think it's ok for me to just remove Float.storeOn entirely and let it call SimpleNumber's? It appears that the current implementation has a similar bug anyway:

c = CollStream.new;
(0/0).storeOn(c);
c.contents; // "nan.0"

[mossheim]

Also, I noticed this comment -

supercollider/SCClassLibrary/Common/Math/Float.sc

Lines 67 to 69 in cca12ff

	// the correct place to implement compileString for Float is in DumpParseNode.cpp
	// int asCompileString(PyrSlot *slot, char *str)
	// for now, solve it here.

Should I use this same method for compileString in the function mentioned? IIUC then I could get rid of Float.asCompileString's special implementation as well.

[telephon]

The question is whether we want the storeOn and the printOn differ at all. If we make printOn print fewer digits, this makes large arrays more readable. But it might also be a little confusing.

If we take the two to be the same, I tend to recommend first to fix the storeOn method using your primitive call, and then implement printOn in terms of it. This makes clear that the number of digits posted are actually enough, and nobody has to wonder whether storeOn is insufficient.

[mossheim]

The question is whether we want the storeOn and the printOn differ at all. If we make printOn print fewer digits, this makes large arrays more readable. But it might also be a little confusing.

I don't understand the difference between these two; I've never used them directly, and I can't find any documentation that explains it at any level of detail. It seems like printOn is to asString what storeOn is to asCompileString. If that is the case, then Float should have the same representation for both.

[telephon]

I didn't realize that these two methods are undocumented. I'll try a short explanation.

Note that many implementations of these two methods could be still improved.

printOn (asString)

produce a string that represents the object close enough for a human reader. By default, this only returns the class name (e.g. a SinOsc), but usually is implemented in a way that the object can be recognized and distingushed (e.g. from other objects of the same class). Infinite collections (e.g. collections that contain themselves) are only partially printed, ending with ellipsis.

The REPL calls printOn (via asString) on the result when code is evaluated.

SinOsc.ar // -> "a SinOsc"

(freq: 600, amp: 0.1) // -> "( 'freq': 600, 'amp': 0.1 )"

Event.default // -> "(  )"

List[1, 2, 3] // -> "List[ 1, 2, 3 ]"

a = [1, 2, 3].add(a); -> "[ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2, 3, [ 1, 2...etc..."

storeOn (asCompileString)

produce a string that, when evaluated, returns an object equal to the object. For example the above Event.default.asCompileString will post the entire parent event.

This can be a way to store objects in a human readable form. Not all objects can be reproduced this way, and for some objects the representation is not possible or otherwise problematic. Then the method asTextArchive is used instead.

[telephon]

If that is the case, then Float should have the same representation for both.

Maybe yes. It would of course be ideal if, for printOn, 7/27 would return not 0.25925925925926 but 0.259… but that might be asked too much.

[aspiers]

Thanks again for all your great work on this one folks! :-)

[mossheim]

Thanks again for all your great work on this one folks! :-)

You're welcome!

Note that many implementations of these two methods could be still improved.

Could we do this in a separate PR @telephon ? This one is all ready to go as-is and I'm still not sure I understand what you want re: storeOn/printOn.

[telephon]

@brianlheim sure!

[jamshark70]

I guess it's a bit of a side note, but this change accidentally broke Ambisonics Toolkit.

ATK looks up impulse responses based in part on the server's sample rate. The server reports the sample rate back to the client as a float. Then ATK does sampleRate = server.sampleRate.asString.

So what used to be a path blah/blah/44100/blah/blah is now blah/blah/44100.0/blah/blah and ATK reports that every possible sample rate is unsupported.

On the one hand, this change is technically sound. I'm not suggesting to revert.

On the other, something is wrong with our process if we all thought that string representations are purely cosmetic, and we all forgot that strings may refer to disk locations that are not flexible, and some parts of the path might be numeric. It bothers me that not a single one of us (myself, too) considered it at all.

[nhthn]

yeah, it's definitely kinda troubling. i've documented it as a breaking change in the 3.10 changelog, so it will at least be very visible in the release notes.

[nhthn]

dang. for the record, seems like we broke another thing: https://www.listarc.bham.ac.uk/lists/sc-users/msg62217.html

like i said in the thread, if your code breaks because of this change, it's probably relying on pretty fragile logic anyway. but either way, we have some lessons to take away from this. no one in particular is at fault, but we all majorly underestimated the impact of this PR.