Fix #3507 #3508
Fix #3507 #3508
Conversation
- renamed test test_format_fixed_field to test_format_fixed_field_string - test_format_fixed_field_string not tests `field_two` with string values - added new test_format_fixed_field_integer to test field `field_one` as INTEGER - added new test_format_fixed_field_integer_normalized to test rounding float values
…treated as `types.INTEGER`
There was a problem hiding this comment.
Nice work! This is really great!! I expected this to break more things, but it looks like the effects on the tests were rather small. As you can see from this tragic inline comment:
Lines 89 to 90 in 545c65d
…we always intended to make the normalization more strict in dbcore, and this is a step in the right direction.
Would you mind adding a quick changelog entry? Then this should be ready to merge.
test/test_autotag.py
Outdated
| @@ -91,7 +93,10 @@ def test_current_metadata_likelies(self): | |||
| for i in range(5)] | |||
| likelies, _ = match.current_metadata(items) | |||
| for f in fields: | |||
| self.assertEqual(likelies[f], '%s_1' % f) | |||
| if type(items[0]._fields[f]) in (Integer, PaddedInt): | |||
There was a problem hiding this comment.
Maybe a slightly easier way to do this would be isinstance(likelies[f], int)?
There was a problem hiding this comment.
yes, it looks simpler.
I am happy you think so. I was surprised to find an issue (#762) from 2014 which already brings this up. But TBH, I am being a bit selfish here in proposing this fix because I need this to work correctly for my new plugin. But hey! Isn't that always like that? ;)
Yes, me too ;) But there are not many tests targeting this. In fact one of the things I wanted to add here is some more tests - I'll look at the coverage and see if I can cover Integers.
Sure. |
|
@sampsyo - this is ready to go! |
|
Looks great! Thank you!! |
Adding normalize method to class
types.INTEGERoverriding the inherited method from the Type class that simply returned the provided value. Also provides Fix #762 and Fix #3507.