so i think this is a good idea overall. i'm a little concerned about adding too many features to this function, but date/time conversion functions tend to be that way, i think by necessity. and there's no reason milliseconds should be the default.

the inverse function asSecs should also be updated to accept strings in this format.

the inverse function asSecs should also be updated to accept strings in this format.

asSecs already accepts any number of floating point digits.

asSecs already accepts any number of floating point digits.

good! then the only thing to update is the documentation :)

Return a Float based on converting a time string in format (ddd):hh:mm:ss.sss

Return a Float based on converting a time string in format (ddd):hh:mm:ss.sss

an example already has this: "-23:12.3456".asSecs;; how would you phrase the explanation to indicate variable number of digits? or would adding more examples be the way to go?

yes i saw that example, but it's in conflict with the wording in the documentation. up to you how to phrase it, but i would say "hh:mm:ss.z", where z can be any sequence of decimal digits. is the fractional part optional? in that case i would notate it as "hh:mm:ss(.z)". also worth checking what happens with "hh:mm:ss." (decimal point but no fractional part) and documenting that too.

@brianlheim I think I addressed your concerns, could you take a look?
("hh:mm:ss." - string with trailing period - is not explicitly documented, but showed in examples)

I also realized that this method breaks with scientific notation. I don't see a way to fix it ATM. Do you think that should be documented, fixed (how?) or is it fine as is, since it's rather an edge case?

9e-05.asTimeString(precision: 0.00001, decimalPlaces: 5)
// -> 00:00:00.9e-05

I also realized that this method breaks with scientific notation. I don't see a way to fix it ATM. Do you think that should be documented, fixed (how?) or is it fine as is, since it's rather an edge case?

that's a total non-starter. there's got to be a way to do this, it's such a basic formatting task. i just checked and even Float.asStringPrec doesn't do it. if we had #3570 we could accomplish this very easily. for now you could just fix it inline, with a note saying that when we solve #3570 (i.e., add sprintf) we can use that directly?

fixed (how?)

https://www.geeksforgeeks.org/convert-floating-point-number-string/

except we only need the code to convert the fractional part

@brianlheim thanks for the feedback and the example code. Just to clarify, my thinking behind this possibly not be a task for this PR was mainly because, as you noted, SC currently doesn't have means of controlling the format of strings obtained from floats. I'll update the PR shortly.

EDIT: PR updated

Just to clarify, my thinking behind this possibly not be a task for this PR was mainly because, as you noted, SC currently doesn't have means of controlling the format of strings obtained from floats.

i understand that point, but we also should only knowingly be shipping incomplete/broken features as a last resort. my order of preferences would be:

1. fix FR: equivalent to printf / sprintf #3570 and then implement this feature
2. implement this feature with a noted workaround for FR: equivalent to printf / sprintf #3570
3. don't implement this feature
4. implement this feature in a broken state

which is, in abstract form:

1. add working feature with no technical debt
2. add working feature with technical debt
3. don't add feature
4. add broken feature

i would order these differently depending on the amounts of technical debt and brokenness involved. my view is, some features we are ok with implementing in an imperfect state, if they maybe require more time or feedback to get right but add a lot of value right away. similarly, we can ship things that add technical debt as long as the route to working it down is clear and easy. here, the workaround is pretty obvious, and the way to iron it out later is also obvious, so i'm backing option (2).

@brianlheim sounds good! Just to be clear, I wasn't attacking your point, just explaining mine.

Does the implementation and the comment look fine for you now?

there was no offense taken, i am a little tired so i'm sorry if that came off as defensive.

it looks good at a glance, i would appreciate if you could add some tests so we can be sure of the behavior.

@brianlheim I think I addressed all your comments and fixed the bug with large values of decimalPlaces. Thank you for finding that! I even added a test for it :)

re: precision @dyfer

like i said, i don't know much about numerical computation / numerical analysis, but i can at least test to see whether an implementation is correct. i've been using this code to test it:

(
n = 6;
~inequalCount = 0;
~rounds = 100000;
postln("real\t\t\t\tasString\tasTimeString");
~rounds.do {
	
	~randVal = 1.0.rand.round(pow(10, n.neg));
	while { ~randVal.asString.contains($e) } {
		~randVal = 1.0.rand
		// weed out ones that get represented as exponential notation
	};
	~timeString = ~randVal.asTimeString(decimalPlaces: n, precision: pow(10, n.neg));
	~string = ~randVal.asString.keep(n + 2).padRight(2 + n, "0"); // 2= num chars for leading "0."
	~timeStringSuffix = ~timeString.keep(n.neg - 2); // 2= num chars for leading "0."
	
	if (~string != ~timeStringSuffix) {
		// pad to 18 to keep columns consistent
		postf("%\t%\t% (%)\n", ~randVal.asString.padRight(18, " "), ~string, ~timeStringSuffix, ~timeString);
		~inequalCount = ~inequalCount + 1;
	}
};

~ratio = ~inequalCount / ~rounds;
~ratio
)

for n=5 there are already a few small errors (click on 'details' to expand):

for n=6 there are also errors where extra digits are appended, it looks like at some points curInt is rounded up to 10:

for n=12 there are too many to print, and the ~ratio at the end is ~0.9999.

the first kind of errors are not so bad to me, but the second (printing 0.771 instead of 0.78) are very very bad.

i looked at an actual implementation of printf and it's horrendously complex: just the floating-point portion of glibc's is over 1000 lines long! (https://github.com/lattera/glibc/blob/master/stdio-common/printf_fp.c#L205)

unless you see a way around it, i'm going to consider this blocked on #3570. it's not a heavily demanded feature, and i don't want to ship broken code.

@brianlheim thanks for a thorough test!

I think I arrived at a reasonable solution. I'm using something like number.frac.asString and then checking for scientific notation and treating the resulting string appropriately.

It does pass your test - although I made a small change: I think the number you compare against should also be rounded:

	~string = ~randVal.round(pow(10, n.neg)).asString.keep(n + 2).padRight(2 + n, "0"); // 2= num chars for leading "0."

Please let me know how this looks now!

EDIT:
I'm thinking whether rounding should be also clamped based on the number of significant figures after floating point in the input number?

I'm thinking whether rounding should be also clamped based on the number of significant figures after floating point in the input number?

I made another change and this time I'm not using .frac which I believe avoids the problem of rounding the fractional part separately

I think the number you compare against should also be rounded:

ah! i'm doing that in the initial assignment to ~randVal, but didn't realize i wasn't doing that inside the while loop. that probably explains some of the problems there. thank you.

i was able to find some failures with n=18:

real				asString	asTimeString
0.0087124109268188	0.008712410926818800	0.008712410926818900 (00:00:00.008712410926818900)
0.0078579187393188	0.007857918739318800	0.007857918739318900 (00:00:00.007857918739318900)
0.0083004236221313	0.008300423622131300	0.008300423622131400 (00:00:00.008300423622131400)
0.0089718103408813	0.008971810340881300	0.008971810340881400 (00:00:00.008971810340881400)
0.0080868005752563	0.008086800575256300	0.008086800575256400 (00:00:00.008086800575256400)
0.0086055994033813	0.008605599403381300	0.008605599403381400 (00:00:00.008605599403381400)

but they're all in the last significant digit which seems totally acceptable to me. it may not even be worth documenting.

as for the branches handling scientific notation.. i think something is wrong here:

0.000000994999.asTimeString(precision: 1e-12, decimalPlaces: 12)
-> 00:00:00.009949990000

at this point, i'd prefer just solving #3570 directly and using that here. this seems like a bit of a rabbit hole. but, if you want to keep working on it, you're welcome to do so.

For the record: I think the scientific notation issue is now solved. The tests for n=18 (and larger) did not yield any errors for me, but I think I made a change that was not pushed with the last commit, sorry.

I'll revisit this once commits from #4662 are in develop. Maybe by that time #3570 will be solved and could be used as a better solution. For now what I have here seems to work, but will probably need more testing.

hey @dyfer -- i'm still interested in merging this one. it looks like #4662 is in develop but #3570 is still unsolved. do you want to revisit this?

Thanks for looking into this @brianlheim

I've rebased this to resolve conflicts. It seems that the current implementation passes all the cases that we've thought of before, but it would be much cleaner if we had an equivalent of sprintf.