[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: The trunk warning was squashed in r73004. -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: I think we're done here. There's only the struct.error to be replaced by OverflowError or TypeError. Do you start the discussion on python-dev? I don't know how to. -- status: pending - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Yes, there should probably be a python-dev discussion. I'll add it to my list of things to do, if you like! And I still have to deal with the original compiler warning that started it all in trunk... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Hmm. In 3.0 and 2.7, I get: from struct import pack pack('L', 123.0) sys:1: DeprecationWarning: integer argument expected, got float b'{\x00\x00\x00' So it looks like we already changed py3k to get rid of the DeprecationWarning. I think the idea was that eventually *only* integers would be accepted for the integer format codes. So pack('L', 123.0) should ideally raise TypeError. If we're not going to do that, we should at least put the DeprecationWarning back in, but it seems better to actually go ahead with the deprecation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Here's an updated version of your patch: all I've done is fix up get_(u)long, get_(u)longlong and get_pylong so that they only accept integers. -- Added file: http://bugs.python.org/file13722/cleanup_float_coerce_patch_v2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Committed part 3 in r71755. Thank you! Is there anything else you think we need to fix up here? -- resolution: - accepted stage: test needed - committed/rejected status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: That should be r71754, of course. -- status: pending - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Changes by Mark Dickinson dicki...@gmail.com: -- status: open - pending ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: Hi Mark, currently there will be no struct.error neither TypeErorr because PyLong_AsLong floors a given float (see my msg84620). The Question is: should I test for long explicitly and raise an error if a different type is given? In this case a test is needed, I give in. I'm not sure what's the right style here. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: Hi, could you have a look at cleanup_float_coerce_patch.diff. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: The _struct.c part of the patch looks good. For the tests, there should be some tests to check that attempting to pack a float with an integer format gives either TypeError or struct.error. Thanks! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Changes by Mark Dickinson dicki...@gmail.com: -- stage: patch review - test needed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: Doesn't took much time. Even nothing happend. PyLong_AsLong trys to convert to int bevor raising an Exception. I'm not sure if struct.pack('l', x) should raise an Exception when a float is given. In case of string there is one. I first did PyLong_Check the parameter and raised an StructError. But I removed this from the patch. I leaved the documentation untouched. Nobody will notice the changes. I increased the version number. Whether we overwork the Errors prior 3.1b or not we could leave it at 0.3. -- Added file: http://bugs.python.org/file13482/cleanup_float_coerce_patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: Yes you're right. The TypeError should be an OverflowError. It was just the copy and paste thing. Hm, I also wondering why struct.error is used. But someone already wanted to change this. The patch looks fine. Do you want to go ahead with the float coercion thing? Maybe I'll find some time the next week to care about. I think there's no hurry. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Thanks again, Andreas. Applied in r70688. There's no particular hurry on removing the float coercion, except that I'd like to get it in before the first 3.1 beta. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Oops. Out-of-date version of the diff. Here's the right one. -- Added file: http://bugs.python.org/file13443/cleanup_range_check_patch2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Changes by Mark Dickinson dicki...@gmail.com: Removed file: http://bugs.python.org/file13441/cleanup_range_check_patch2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Changes by Mark Dickinson dicki...@gmail.com: -- priority: - normal stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Thanks for the patch. A couple of questions and comments: (1) at line 300-ish of test_struct, should (struct.error, TypeError) be (struct.error, OverflowError)? I don't think out-of-range values should be raising TypeError. If they are, perhaps we should change that. (2) It looks like the deprecated_err function isn't needed any more (yay!); let's remove it. (3) I'd prefer to keep the test_1229380 bit, but just replace the deprecated_err with an assertRaises, just like you already did further up. As far as I can see those tests aren't entirely duplicated by others, and one can never have too many tests... -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: I removed definition of _PY_STRUCT_RANGE_CHECKING. The test_standard_integers now expects struct.errors when there are out of range values (should have been part of my last patch). So test_1229380 is now obsolete. -- Added file: http://bugs.python.org/file13401/cleanup_range_check_patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Changes by Andreas Schawo andreas.sch...@gmail.com: Removed file: http://bugs.python.org/file13304/struct_overflow_patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: Thanks! Committed your patch in r70497. I think _PY_STRUCT_RANGE_CHECKING can also be removed from the module and the tests (treated as though it's 1 throughout). In theory there could be people using it, but it's not documented and the leading underscore in the name clearly indicates that its supposed to be private, so I think it's safe to remove. It looks like it was there just to enable some extra tests (tests that were broken with earlier versions of the struct module thanks to struct bugs). I agree that we should bump the version number once we're done here. I really appreciate this work: the struct module has been in need of cleanup for a while. Thank you again. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: Ok, heres the patch again. Passed regression tests. Should the version number increase to 0.3? Maybe to reflect there are no more deprecated features. It should then take place after FLOAT_COERCE cleanup. I'm currently trying to figure out whats about _PY_STRUCT_RANGE_CHECKING and why it is hardcoded constant in _struct.c. -- Added file: http://bugs.python.org/file13384/struct_overflow_patch.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Mark Dickinson dicki...@gmail.com added the comment: I think you're right about issue 4228: we should go ahead and clean up the struct module anyway. As far as I can tell, only the Python 2.5 zipfile module is using the deprecated behaviour. The _struct.c portion of your patch looks fine. I think there's still some work to do on the test_struct file: for example, removing references to PY_STRUCT_OVERFLOW_MASKING. I'd also like to remove the deprecated float handling, but it's probably better to do that in a separate patch once this patch is in. Changing title to better reflect the current patch. -- title: Compiler warning get_ulong is never used 3.x - Remove deprecated features from struct module type: compile error - feature request ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue5463] Remove deprecated features from struct module
Andreas Schawo andreas.sch...@gmail.com added the comment: I agree with you. A separate patch will do better. In the next days I'm able to provide a new patch with the test_struct cleanup. Currently all tests succeeded on Windows and Linux. So I think no other module relies on this feature explicitly. I've already checked the documentation, but there are no references to overflow masking. So there's nothing to do for now. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue5463 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com