[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Roundup Robot added the comment: New changeset 29becac5dd9b by Stefan Krah in branch '3.3': Issue #15783: Except for the number methods, the C version of decimal now http://hg.python.org/cpython/rev/29becac5dd9b -- nosy: +python-dev ___ Python tracker

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: New patch with docs. I've eliminated the unused context arg from canonical(). I can't possibly imagine that anyone uses it (or that anyone uses canonical() at all for that matter). -- Added file: http://bugs.python.org/file28009/issue15783-all-2.diff

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: OK, here's a patch. Specifics: o Except for the number methods, decimal.py and _decimal should behave identically now. o _decimal actually requires an additional context arg in same_quantum(), compare_total(), compare_total_mag() and copy_sign(). This

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Mark Dickinson added the comment: > Should we go ahead with this? Sure: if you're willing to write the patch, go for it. -- ___ Python tracker ___ _

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: By "all cases" I mean the ones listed in msg169144, not the Number methods. -- ___ Python tracker ___ _

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: Should we go ahead with this? If so, then I think it only makes sense to change *all* undocumented cases of None default args. Here's a case that used rounding=None in quantize(): https://bitbucket.org/simplecodes/wtforms/issue/117/test-failure-with-python-33

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Nick Coghlan added the comment: I agree it's an acceptable risk for 3.3.0. Most third party decimal operations simply won't accept an explicit context argument at all - avoiding the need to pass the current context around explicitly is the whole point of it being stored in the thread locals.

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Mark Dickinson added the comment: I don't really feel that strongly either way, either. I understand Nick's arguments, but can't help feeling that we're letting a doc mistake dictate the API here. (And my inner type-system has an aversion to APIs where arguments are expected to be *either* N

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: I don't want to block this, BTW. Personally I'm +-0 on the issue. If more people think this is needed for backwards compatibility, I'll write a patch for localcontext. But I'd rather do that *very* soon. FWIW, none of the cdecimal users has ever complained about t

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: Nick Coghlan wrote: > Any third party Decimal manipulating function that accepts an > optional context and passes it down to the standard Decimal API > will be confronted with the same problem in 3.3: passing None > as the context no longer means the same thing as

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Nick Coghlan added the comment: Note that it's only the 3.2 interactive help that specifies "ctx=None", and it's almost certainly lifting that directly from the Python implementation. The C implementation is definitely in compliance with the prose docs, which give the signature as localcontext

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Changes by Zachary Ware : -- nosy: +zach.ware ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pytho

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Changes by Arfrever Frehtes Taifersar Arahesis : -- nosy: +Arfrever ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscri

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Mark Dickinson added the comment: Adding Raymond to the nosy, since he may know more about what was intended. -- nosy: +rhettinger ___ Python tracker ___

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Mark Dickinson added the comment: This doesn't strike me as something it's essential to resolve before 3.3.1. And I agree that decimal.localcontext(None) looks peculiar. I noticed this yesterday when looking at #15136 and dismissed it as an artifact of the implementation---I didn't think to c

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: This is the complete list of context=None divergences. It would be possible to change that, since most of the functions are generated by macro wrappers. # This is the only place where 'ctx' is used localcontext(ctx=None) canonical(self, context=None) compare(self

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Changes by Mark Dickinson : -- nosy: +mark.dickinson ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mai

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: > Can't this be fixed in the CONTEXT_CHECK_VA macro? No, the macro should only allow contexts. E.g. in the case of localcontext(None) context_copy(local, NULL) would be called on local=Py_None, which is undefined. What would perhaps be needed for a nice C-API is a

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: OK, but for example: Help on function __truediv__ in module decimal: __truediv__(self, other, context=None) Return self / other. Here I think it's undisputed that a C version should not cram a context argument into a number method. There are many functions in

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Ezio Melotti added the comment: Can't this be fixed in the CONTEXT_CHECK_VA macro? With the attached patch decimal.localcontext(None) works and there aren't any failures, however I don't think there are tests that pass None explicitly, so this approach might not work. -- keywords: +pat

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Nick Coghlan added the comment: While it's undocumented in the main docs [1], help(decimal.localcontext) in 3.2 starts with: localcontext(ctx=None) Return a context manager for a copy of the supplied context We essentially have two votes in favour of "it should work" (the pure Python impl

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Stefan Krah added the comment: The feature is undocumented though. Is it a regression? There are other places as well. The topic was raised in msg153447 and msg153506, with no further comments. So I concluded that None argument support wasn't important. -- _

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

Nick Coghlan added the comment: Georg, a regression in the decimal API relative to 3.2 got picked up by #15136. Are you OK with cherrypicking a fix for this into rc2? -- nosy: +georg.brandl ___ Python tracker

[issue15783] decimal.localcontext(None) fails when using the C accelerator module

New submission from Nick Coghlan: decimal.localcontext() works correctly with both the pure Python decimal module and the C accelerator. decimal.localcontext(None) only works with the pure Python version. The C accelerator throws an exception instead of treating it as equivalent to omitting t