Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: repeated dict keys in formatting testing results in some keys going untested. #162

Open
jagerber48 opened this issue Oct 14, 2022 · 1 comment

Comments

@jagerber48
Copy link
Contributor

See for example

https://github.com/lebigot/uncertainties/blob/804adccf3401aeacbcbae0d669f92131fcd02c03/uncertainties/test_uncertainties.py#L1596-L1607

These are two ufloat value entries in test_format. Since the two dict entries have the same keys the latter overwrites the former and the cases in the former do not get tested.

Immediate solutions I see:

  • Change the value/uncertainty pairs so there are no duplicates (hard to enforce over the long block of code)
  • Add a "uid" element to the value/uncertainty tuple that could enumerate multiple dict entries meant to test against the same value/uncertainty pair (also hard to keep problem-free throughout the long block of code
  • Reformat the construction of the dict so not constructing as a literal. Instead construct with something like:
tests = dict()
u_pi_pair = (3.1415, 0.0001)
tests[u_pi_pair] = dict()
tests[u_pi_pair]['*^+7.2f'] = '*+3.14*+/-*0.00**'
...

Not the most elegant probably but would at least fix this issue.

There are likely other possibilities.

@jagerber48 jagerber48 changed the title Bug: dict keys in formatting testing results in some keys going untested. Bug: repeated dict keys in formatting testing results in some keys going untested. Oct 14, 2022
@jagerber48
Copy link
Contributor Author

See PR: #167 I made a PR to convert the dict of dicts of test cases into a list of tuples of test cases and expected results. After making this change some test cases were "uncovered". Some of these failed at an initial test. I went through and modified the unit tests until they all pass. Most or all of the test cases that were failing look like oversights in the construction of the unit tests but I'm not sure. Here's a summary of the changes needed to pass:

line 1568: (3.1415, 0.0001), '>10f', '  3.141500+/-  0.000100' 
should return '   3.14150+/-   0.00010' 
Default formatting of 'f' in uncertainties package uses pdg sig figs on uncertainty to determine precision, not python default of 6 digits after decimal point.

line 1570: (3.1415, 0.0001), '0.4e', '3.1415e+00+/-0.0000e+00'
should be '1.4e', '3.1415e+00+/-0.0001e+00'
First, the uncertainty 0.0001 shouldn't be rounded to 0 at precision of 4. Second, the `__format__()` documentation indicates a width of 1, not 0, must be specified to force double exponent.

line 1598: (12.3, 456.78), '', '12+/-457'
should be '(0+/-5)e+02'. I think no formatting defaults to PDG sig fig on uncertainty to determine precision. Seems like in this case scientific notation is also used by default. I can't tell if that is expected or not.

line 1640: (1234.56789, 0.1), '.0f', '(1234+/-0.)'
should be '1235+/-0'
First, rounding on the "4" in the ones place was missed. Second, as noted in a docstring comment in the code, '.0f' apparently does not put a decimal on the end of floats. This causes a conflict with the uncertainty packages' convention that 0 means no error while 0. means non-zero error but which has been rounded to zero. Not sure how this is expected to be handled. Third parentheses shouldn't be included?

line 1641: (1234.56789, 0.1), 'e', '(1.23456+/-0.00010)e+03'
should be '(1.23457+/-0.00010)e+03'
Forgot to round

line 1642: (1234.56789, 0.1), 'E', '1.23456+/-0.00010)E+03'
should be '(1.23457+/-0.00010)E+03'
Forgot to round

line 1647: (1234.56789, 0.1), '%', '123456+/-10%'
should be '(123457+/-10)'%
missing parentheses

I can't tell if my updated expected values are in line with what is actually expected according to the documentation within the package documentation and within the code. I think most of the cases are benign just having to do with what default settings are for things like precision or the presence of parentheses when nothing is specified. Lines 1598 and 1640 are the most alarming to me since the first one had a big change from what was originally expected and the 1640 line has to do with a thorny edge case about decimals after integers.

jagerber48 added a commit that referenced this issue Aug 9, 2024
…rwrites (#167)

This PR

- Moves formatting code into a `test_formatting.py` module
- Refactors `test_format` to use `pytest.mark.parametrize`. This solves
#162 since cases are no longer stored as dict keys (which were
accidentally repeated). Now each case is a 4-tuple. Previously test
cases with the same value/uncertainty had the option to be grouped
together within a dict under that value/uncertainty tuple. That option
is lost since I don't really know how to do that in `pytest`. I've done
this using `unittest.subTest` in the past, I think there might be a
`pytest` extension that does something like this.
- Removes checks and switch case for jython. Let me know if these need
to be added back in.
- Correction to some tests that were previously not running, but which
are now failing <<add more detail here>>

There is a lot of cruft in comments and possibly tests about subtleties
between old versions of python. Hopefully this can be removed soon.
____
OLD DESCRIPTION:

See #162.
`tests` object containing test cases for ufloat string formatting is
currently a dict where keys are tuples of value/uncertainty pairs and
values are dicts of format string/expected output pairs. Similar tests
are chunked together under the same value/uncertainty pair. However, the
same value/uncertainty pair appears multiple times in the top level
dict. In this case only one grouping of tests is captured in the dict,
and all others are overwritten.

To avoid this overwriting we can convert the dict of dicts into a list
of tuples. In this case elements of the list are tuples whose first
element is a value/uncertainty pair tuple and whose second element is a
dict of format string/expected output pairs.
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

No branches or pull requests

1 participant