Deprecate local_context()?

[skirpichev]

Ordinary context constructors could be used instead, isn't?

For example:

>>> with context() as ctx:
...     ctx.allow_complex = True
...     print(sqrt(-2))
... 
0.0+1.4142135623730951j
>>> ctx = get_context()
>>> ctx.allow_complex
False
>>> print(sqrt(-2))
nan

[casevh]

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.

[casevh]

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.

[skirpichev]

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...

[casevh]

Gmpy2 originally had a separation between context and context manager. It has some random, rare bugs. When I converted to using contextvars, a separate context manager is no longer needed. So local_context needs to be rewritten and the gmpy2 context manager can go away.

On Sat, Nov 11, 2023, 11:02 PM Sergey B Kirpichev @.***> wrote:

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...

— Reply to this email directly, view it on GitHub https://github.com/aleaxit/gmpy/issues/454#issuecomment-1807024452, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMR2332VF3N64P3APZX75DYEBYBRAVCNFSM6AAAAAA7AU2G4WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGAZDINBVGI . You are receiving this because you commented.Message ID: @.***>

[casevh]

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

[skirpichev]

For some history of the localcontext():

• https://github.com/python/cpython/commit/89f507fe8c497b3f70fdcecce8bc240f9af2bbe2 (implementation)
• https://mail.python.org/archives/list/python-dev@python.org/thread/RAQWSFMOZ3T56U46MM4VOUFGQNK3YW4X/#SEPQSMTEEYGASBYCV3X3UPVNL6VM4JQT (issues with the prior implementation)

[casevh]

Current status

1) I've modernized local_context. It should be a bit cleaner but I'll probably revisit it. 2) My first attempt at using a context also as a context manager failed due to the same issues found in the python-dev chat. I have another plan.

More comments after I complete (2).

On Fri, Nov 17, 2023 at 9:39 PM Sergey B Kirpichev @.***> wrote:

For some history of the localcontext():

• @.*** https://github.com/python/cpython/commit/89f507fe8c497b3f70fdcecce8bc240f9af2bbe2 (implementation)

• @.***/thread/RAQWSFMOZ3T56U46MM4VOUFGQNK3YW4X/#SEPQSMTEEYGASBYCV3X3UPVNL6VM4JQT (issues with the prior implementation)

— Reply to this email directly, view it on GitHub https://github.com/aleaxit/gmpy/issues/454#issuecomment-1817400987, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMR236CDIWTV7EVENQ5GKTYFBCXRAVCNFSM6AAAAAA7AU2G4WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJXGQYDAOJYG4 . You are receiving this because you commented.Message ID: @.***>

[casevh]

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.

[casevh]

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.

[skirpichev]

On Sat, Dec 02, 2023 at 09:35:49PM -0800, casevh wrote:

The original context was never restored. That has been fixed.

Yes, I've seen this while reading code after discussion above.

More comments to follow

BTW, if you have some work in progress - you also could create a PR for this. It's possible to comment on commits in the master, but this will be less discoverable for other people in future.

let's not not decide on the future of local_context just yet.

We should keep this in some form for a while, for compatibility reasons.

[skirpichev]

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.

[casevh]

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.

[skirpichev]

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.

[casevh]

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 changelocal_context()` to return a context object instead of a context manager object. Should I do this and also remove the context manager type?

[skirpichev]

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?

[casevh]

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.

[skirpichev]

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.

[skirpichev]

See #460