FEAT-#7141: Add an ability to use config variables with a context manager

[dchigarev]

What do these changes do?

This PR enables for this behavior to be possible:

import modin.config as cfg

print(cfg.RangePartitioning.get()) # False

with cfg.update_config(RangePartitioning, True):
    print(cfg.RangePartitioning.get()) # True

print(cfg.RangePartitioning.get()) # False

• first commit message and PR title follow format outlined here

  NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

• passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
• passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
• signed commit with git commit -s
• Resolves Add an ability to modify config variables using context manager #7141
• tests added and passing
• module layout described at docs/development/architecture.rst is up-to-date

[anmyachev]

Asserts may be disabled in user code; it is better to throw ValueError exception.

[AndreyPavlenko]

Are there (or could be in the future) any parameters that support None value?

[dchigarev]

good point, it's better to use some nodefault placeholder instead

[AndreyPavlenko]

You can check isinstance(config, dict) first. The config parameter could be either dict or Parameter.

[AndreyPavlenko]

I've 2 minor suggestions:

1. The function name update() seems not intuitive to me. From this name I would think that it's intended for the config update, but it's rather a config context switch than update. What about context() or override()?
2. With the current approach we have different syntax for a single and multiple values:

with cfg.update(cfg.RangePartitioning, True):
    ...
with cfg.update({cfg.RangePartitioning: True, cfg.NPartitions: 1, cfg.MinPartitionSize: 64}):
    ...

What about something like this?

with cfg.context(RangePartitioning=True):
    ...
with cfg.context(RangePartitioning=True, NPartitions=1, MinPartitionSize=64):
    ...

It seems more consistent and compact.
A simple implementation:

# modin/config/__init__.py
modin_config = sys.modules[__name__]

@contextlib.contextmanager
def context(**configs):
    for k, v in configs.items():
        config = getattr(modin_config, k)
        configs[k], _ = config.get(), config.put(v)
    try:
        yield
    finally:
        for k, v in configs.items():
            getattr(modin_config, k).put(v)

[dchigarev]

The function name update() seems not intuitive to me.

context and override also don't seem intuitive to me :D Renamed to update_config

What about something like this?

with cfg.context(RangePartitioning=True):
    ...
with cfg.context(RangePartitioning=True, NPartitions=1, MinPartitionSize=64):
    ...

It looks good, but it limits even more on how users can apply this. For example, one must either use a string name to refer to a config or import it separately, they can't refer to a config using cfg.Variable:

with cfg.context(cfg.RangePartitioning=True): # doesn't work
# File "<stdin>", line 1
#    cfg.context(cfg.RangePartitioning=True)
#                ^
# SyntaxError: expression cannot contain assignment, perhaps you meant "=="?

# one should either do:
with cfg.context("RangePartitioning"=True):
    ...

# or import the config variable separately
from modin.config import RangePartitioning
with cfg.context(RangePartitioning=True):
    ...

I don't like this type of limitation

[AndreyPavlenko]

or import it separately

No, you don't need any additional imports. The code I provided works as is, the only import is required - import modin.config as cfg

[AndreyPavlenko]

Renamed to update_config

update_config seems unintuitively redundant :)

import modin.config as config

with config.update_config(RangePartitioning=True):
    ...

vs

import modin.config as config

with config.context(RangePartitioning=True):
    ...

vs

import modin.config as config

with config.override(RangePartitioning=True):
    ...

[dchigarev]

or import it separately

No, you don't need any additional imports. The code I provided works as is, the only import is required - import modin.config as cfg

Agree, but it still doesn't allow you to write with context(cfg.RangePartitioning=...)

[AndreyPavlenko]

Agree, but it still doesn't allow you to write with context(cfg.RangePartitioning=...)

It doesn't, but do you really need it? with context(RangePartitioning=...) seems more convenient.

[AndreyPavlenko]

Does it make sense moving this function to the modin.config module?

[dchigarev]

you mean to modin/config/__ini__.py? Idk whether we should define something without a strong reason in an __init__.py file.

[AndreyPavlenko]

Yes, but it's up to you. If you think the current location is better, I don't mind.
LGTM!

[anmyachev]

LGTM! @AndreyPavlenko any more comments?