Skip to content

Conversation

@jfinkels
Copy link
Collaborator

This pull request fixes a bug in the behavior of seq with the -w option when printing floating point numbers with digits after the decimal point. Previously, the code assumed that the string provided on the command-line was a floating point number in decimal format, like 12.34. But the command-line argument might be a floating point number in scientific notation, like 1e-2. This commit changes the code to compute the number of required decimal places to display in fixed-width mode after converting the command-line argument to f64. For example:

$ seq -w .1 1e-2 .11
0.10
0.11

@jfinkels jfinkels force-pushed the seq-width-fractional branch from 1d801cd to 21b7d64 Compare August 31, 2021 00:17
@miDeb
Copy link
Contributor

miDeb commented Aug 31, 2021

As I understand it this moves the logic to calculate the number of fractional digits from being based on the user input to a stringification of the parsed user input. This is a difference if you add trailing zeros to your example: seq -w .1000000000 1e-2 .11.
The output should be

0.1000000000
0.1100000000

but it is

0.10
0.11

I think what we need to do to match gnu's behavior for scientific notation is to

  • count the amout of digits between the . and the e
  • if there is a + sign after the e subtract the following number from the number of fractional digits
  • if the sign is - add the following number to the number of fractional digits

For example, seq -w .0 1.500e-1 .1 prints 0.0000 , because there are 3 fractional digits in the input and you add one because of e-1.

@jfinkels
Copy link
Collaborator Author

jfinkels commented Sep 1, 2021

You and the other uutils team members are really good at coming up with test cases. 😄 Thank you! I'll add some more test cases and make the suggested changes.

@jfinkels jfinkels force-pushed the seq-width-fractional branch from 21b7d64 to 20495db Compare September 11, 2021 17:00
@jfinkels
Copy link
Collaborator Author

Okay, I have updated this pull request with some more unit tests and a different implementation. At @miDeb's suggestion, we are now parsing the input string itself to compute the number of integral digits and the number of fractional digits needed to display a given number.

This branch includes the changes from pull request #2657, so that one should be reviewed first.

@jfinkels jfinkels force-pushed the seq-width-fractional branch 2 times, most recently from 110037a to 1624dcc Compare September 17, 2021 02:17
Change the way `seq` computes the number of digits needed to print a
number so that it works for inputs given in scientific notation.
Specifically, this commit parses the input string to determine whether
it is an integer, a float in decimal notation, or a float in scientific
notation, and then computes the number of integral digits and the number
of fractional digits based on that. This also supports floating point
negative zero, expressed in both decimal and scientific notation.
Remove the `Number::num_digits()` function in favor of the
`digits::num_integral_digits()` functions.
@jfinkels jfinkels force-pushed the seq-width-fractional branch from 1624dcc to 3854a97 Compare September 18, 2021 03:50
@jfinkels
Copy link
Collaborator Author

Ignore my previous comment about including the changes from another pull request. Everything you need to review this is in this branch.

@sylvestre
Copy link
Contributor

Looks great, thanks

@sylvestre sylvestre merged commit 77cfafa into uutils:master Sep 18, 2021
@jfinkels jfinkels deleted the seq-width-fractional branch January 25, 2022 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants