Deprecate local_context()? about gmpy HOT 18 CLOSED

I think we should deprecate local_context(). But local_context() does support a capability that is missing in context(). local_context() accepts a context object as an optional first argument. If the argument is missing, local_context() uses the current context. context() always creates a new context object. I think we can change context() to accept an optional first argument that defaults to a new context object. Then context(get_context(), **kwargs).

I've always liked using the keyword arguments to create or customize a context object.

I will do some experimentation.

from gmpy.

I just reviewed the docs on the decimal module. The decimal module maintains a distinction between a context object and a context manager. decimal.localcontext() is used in with... statements. I mostly followed the decimal module when I added contexts.

I still like my previous suggestion:

gmpy2.context() returns a new default context object.
gmpy2.context(**kwargs) returns a new default context object that is updated with **kwargs.
gmpy2.context(context_object, **kwargs) creates a copy of context_object that is updated with **kwargs.
gmpy2.context(gmpy2.get_context(), **kwargs) replicates the existing local_context().

Back to experimenting.

from gmpy.

The decimal module maintains a distinction between a context object and a context manager.

Yes. I'll try to dig to the history for reasons of such design.

But gmpy2's contexts are context managers...

from gmpy.

from gmpy.

The current status of context manager in gmpy2 is that is broken in some subtle ways. I am working on a rewrite.

from gmpy.

For some history of the localcontext():

• python/cpython@89f507f (implementation)
• https://mail.python.org/archives/list/[email protected]/thread/RAQWSFMOZ3T56U46MM4VOUFGQNK3YW4X/#SEPQSMTEEYGASBYCV3X3UPVNL6VM4JQT (issues with the prior implementation)

from gmpy.

from gmpy.

This is is getting more complicated. I found some errors on context handling in 2.1.5 and probably earlier versions. An example where the previous context does not get restored. Here is an example:

Python 3.11.6 (main, Oct  8 2023, 05:06:43) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from gmpy2 import *
>>> get_context().precision
53
>>> with ieee(128) as ctx:
...   pass
... 
>>> get_context().precision
113
>>> set_context(context())
>>> get_context().precision
53
>>> with local_context(ieee(128)) as ctx:
...   pass
... 
>>> get_context().precision
53
>>> 

with context doesn't properly restore the previous context but with local_context(context) does.

I'll merge your PR that updated the context tests. Then I will add tests for all the failures that I find and verify they are fixed int the revised context_vars code.

from gmpy.

I have with context and with local_context functioning properly.

with context had a major bug for its entire existence (2.0.0 to 2.1.5). The original context was never restored. That has been fixed.

I will be running more tests but the code is a definite improvement.

More comments to follow so let's not not decide on the future of local_context just yet.

from gmpy.

from gmpy.

Few remarks on latest changes.

1. Section "Nested contexts and coding style".

   "The with ... as ctx: coding style is commonly used. It does not behave as expected with nested contexts." seems to be a bit misleading. This behaviour is entirely within the Python's scoping rules and the with statement specification.
   The binding of the ctx in "with ieee(128) as ctx" will shadow binding above ("with ieee(64) as ctx"). I think our users can expect this from the language docs or the PEP 343 (https://peps.python.org/pep-0343/#specification-the-with-statement). They could use different variables for nested blocks (e.g. "with ieee(128) as ctx2" in your example), not just get_context().
   I think this section could be removed.

2. "Comments on the ieee context"

   I think this section could be removed as well: we have above API docs for gmpy2.ieee (just a function docstring), that says: "Return a new context corresponding to a standard IEEE floating point format. The supported sizes are 16, 32, 64, 128, and multiples of 32 greater than 128."
   Maybe we should mention instead ieee() function in the leading paragraph of the contexts.rst.

3. The documentation says about the current version of the gmpy2, so - we could omit version reference in "Contexts and the with statement". Maybe we should use versionadded/versionchanged directives in sphinx docs?

4. Probably we should emphasize the second example in mentioned section (i.e. with context()). Using local_context() does make sense for me only in it's second form, i.e. with local_context(copied_context, foo=bar) as ctx. Maybe the context() can support this signature? Then local_context() will be a deprecated alias.

I'll play with the new code a bit and, probably, will come with a patch.

PS: See also https://bigfloat.readthedocs.io/. They control precision and rounding mode slightly differently than the decimal module.

from gmpy.

1. Section "Nested contexts and coding style".
   "The with ... as ctx: coding style is commonly used. It does not behave as expected with nested contexts." seems to be a bit misleading. This behaviour is entirely within the Python's scoping rules and the with statement specification.
   The binding of the ctx in "with ieee(128) as ctx" will shadow binding above ("with ieee(64) as ctx"). I think our users can expect this from the language docs or the PEP 343 (https://peps.python.org/pep-0343/#specification-the-with-statement). They could use different variables for nested blocks (e.g. "with ieee(128) as ctx2" in your example), not just get_context().
   I think this section could be removed.

Let's remove my initial comment(s). Can we have examples that show both approaches - get_context() and different variable names?

2. "Comments on the ieee context"
   I think this section could be removed as well: we have above API docs for gmpy2.ieee (just a function docstring), that says: "Return a new context corresponding to a standard IEEE floating point format. The supported sizes are 16, 32, 64, 128, and multiples of 32 greater than 128."
   Maybe we should mention instead ieee() function in the leading paragraph of the contexts.rst.

Mentioning ieee() in the leading paragraph would be better.

3. The documentation says about the current version of the gmpy2, so - we could omit version reference in "Contexts and the with statement". Maybe we should use versionadded/versionchanged directives in sphinx docs?

We should should state that version 2.2.0 fixed a bug in previous versions. versionchanged sounds fine to me.

4. Probably we should emphasize the second example in mentioned section (i.e. with context()). Using local_context() does make sense for me only in it's second form, i.e. with local_context(copied_context, foo=bar) as ctx. Maybe the context() can support this signature? Then local_context() will be a deprecated alias.

I'll change context() to accept a context object. context(get_context()) will then be equivalent to local_context(). Let's just document the context() as the recommended function to use. And update the docstring for local_context() to recommend context(get_context()). I'm okay if local_context() is deprecated but removing it probably isn't worth breaking old code.

I'll play with the new code a bit and, probably, will come with a patch.

from gmpy.

Can we have examples that show both approaches - get_context() and different variable names?

Sure, we can. But why we want this? There is nothing special with contexts in the gmpy2 - that's just how the "with" statement and scoping rules work.

I think we can rely here on some prior knowledge about the Python language from our users. E.g. the decimal module has not similar examples.

We should should state that version 2.2.0 fixed a bug in previous versions. versionchanged sounds fine to me.

We also could mention instead this in release notes. The drawback of sphinx directives is that they could be added only to sphinx docs (of course, we could change the 'context()' docstring, but then help(context) will look ugly in console).

And update the docstring for local_context() to recommend context(get_context())

I think we could point to the right alternative in the warning.

from gmpy.

Please update the docs as you see fit.

I've updated context() to accept a context object. I haven't updated any documentation since I don't know how to document both context and context manager' types. I can easily change local_context()` to return a context object instead of a context manager object. Should I do this and also remove the context manager type?

from gmpy.

Should I do this and also remove the context manager type?

Yes, I believe we can do this. In fact, probably local_context alias (with a deprecation) could be added in the gmpy2/__init__.py. What do you think?

from gmpy.

Should I do this and also remove the context manager type?

Yes, I believe we can do this. In fact, probably local_context alias (with a deprecation) could be added in the gmpy2/__init__.py. What do you think?

I will work on removing the context manager type. I'm fine with documenting context as the recommended replacement for local_context. I'm just not convinced that removing 'local_context' is worth the disruption to (most|almost all) existing gmpy2 code that use contexts.

from gmpy.

I'm just not convinced that removing 'local_context' is worth

No, I'm not suggesting this. Lets just deprecate this interface with an appropriate warning.

from gmpy.

See #460

from gmpy.