Florent Xicluna <la...@yahoo.fr> added the comment:
Thank you for the review and your comments.
Here, the replies.
>
> Amaury review - msg98491
>
> Here is my review of issue7092_syntax_imports_v3.diff:
>
> - test_itertools.py: please replace
> [tuple([arg[i] if i < len(arg) else None for arg in args])
> for i in range(max(map(len, args)))]
> by something more readable (nested for loops for example)
It is the 3.x syntax.
--> No change
> - test_mailbox.py: why doesn't test_support.import_module('rfc822')
> specify deprecated=True? maybe the module can be imported normally...
Added the "deprecated=True"
> - test_pyclbr.py: replace self.assertTrue(key in obj) with assertIn()
> - test_wsgiref.py: replace self.assertTrue(key in h.environ)
> - test_queue.py: test could be identical to the one in py3k.
Thanks
--> Done
>
> Ezio review - msg98494
>
> Here's mine about issue7092_check_warnings_v3.diff:
>
> 1) test_callable should keep testing callable() and the warnings should be
> caught;
Done
> 2) in test_bsddb3 the problems should be correct in the module if possible
> and worth it (the module is deprecated);
Ok. I prepare a separate patch to fix bsddb3
> 3) next to the several '# Silence py3k warnings' it would be nice to have a
> note about what warning you are exactly silencing;
> 4) def test_deprecated_builtin_map -> test_deprecated_builtin_map_with_None,
> otherwise it seems that map is deprecated;
Done
> 5) in test[_deep]_copy I'm not entirely sure that the tests are equivalent
> using in (and if they are you should use assertIn);
Will use assertSetEqual, no need to reinvent the wheel
> 6) in test_socket I would keep callable, also shouldn't the raise in the next
> line raise a warning as well?;
Ok to keep callable. For the next line, I do not see the warning.
> 7) the self.assertEqual(`u2`, `d2`) in test_userdict could just use repr()
> instead;
Done
> 8) a few tests in test_weakref should use assert[Not]In instead of
> assertTrue(x [not] in y).
Done
>
> Ezio review - msg98507
>
> A couple of comments about issue7092_syntax_imports_v3.diff too:
> 1) in test_copy you remove (k,v), but left the name 'k' even if now it
> represent the item and not the key;
Changed 'k'-> 'pair' (like 3.x)
> 2) in test_fractions you should probably use self.fail() instead of an assert;
The surrounding methods use "assert "
--> No change
> 3) on test_ftp you can use two separate lines and remove the ';';
Done
> 4) in test_pyclbr there's one extra 'f' in the comment in assertHaskey;
It is not an extra f. 'iff' = "if and only if" (not obvious for non-English
people)
> 5) in test_xml_etree_c you can either leave callable() and catch the warning
> or use isinstance(x, collections.Callable) instead (there are also a few more
> places where callable was used too).
It is done this way in 3.x.
I propose to fix it separately (if a fix is required).
Now I prepare the updated patches.
I follow the proposal of Amaury: create a specific
"test_support.silence_py3k_warnings()" context manager.
It replaces the "check_warnings" and "filterwarnings" of the proposed patches.
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue7092>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
