chore: deprecation warnings for determined.common.experimental imports

[tayritenour]

MLG-381

Description

Adds deprecation warnings for users who try to import externally from determined.common.experimental. You should be importing from determined.experimental instead

Test Plan

$ python3
>>> from determined.common.experimental import Determined
<stdin>:1: FutureWarning: The 'determined.common.experimental' module is deprecated and will be removed in future versions. Please import from 'determined.experimental' instead. Example: `from determined.experimental import Determined`

Reset Terminal

$ python3
>>> from determined.experimental import Determined
>>> 

Commentary (optional)

Checklist

• Changes have been manually QA'd
• User-facing API changes need the "User-facing API Change" label.
• Release notes should be added as a separate file under docs/release-notes/.
  See Release Note for details.
• Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

[wes-turner]

I like this implementation! And thanks for chasing down the parts of the CLI where the old path was used.

[wes-turner]

How did this get by isort? Though warnings is only needed in the subsequent warnings.warn call, I don't think it's necessary to delay its import until its use.

[tayritenour]

I can move it. Not sure, maybe there are exceptional cases for __init__

[rb-determined-ai]

__init__ is ignored by isort because the way we do our code layout sometimes requires explicit dependency resolution in __init__.py files... meaning it's often not safe to let isort touch them.

[wes-turner]

@rb-determined-ai: could you please comment on this message and expected plan in particular? I continue to add code to the SDK, which itself is entirely contained within determined.common.experimental. I know (hope?) that the code won't live in experimental forever, but maybe it's more accurate to say that (currently) calls into determined.common.experimental from outside are deprecated?

Or should I also add "migrate the SDK away from d.c.e" to the roadmap for the project?

[rb-determined-ai]

determined.common is 100% internal and no end-users should ever use it, ever.

There is a boring legacy reason why this code was originally where it was.

The reason I didn't deprecate it the symbols in d.c.e when I added the experimental.client module was because I expected that we'd be promoting the python sdk out of experiment like... years ago... so I figured I'd do all the breaking changes in one go, no need for waves of deprecation.

In reality, the python sdk has always been half-baked until we put real engineering resources on it this quarter, so it wasn't ever promoted.

My conclusion is: external users running really old code might be using d.c.e and it's worth having a warning.

Internal users can write in d.c.e today and move it tomorrow without any warning and that's fine. External users of today should be accessing all these symbols from det.experimental.client and in the future from det.client regardless of where new symbols are added.

[wes-turner]

What do you think of adding a category=FutureWarning to the function call?

[tayritenour]

I don't see that on my warnings module? The init on the version I have is as such:

class catch_warnings(object):

    """A context manager that copies and restores the warnings filter upon
    exiting the context.

    The 'record' argument specifies whether warnings should be captured by a
    custom implementation of warnings.showwarning() and be appended to a list
    returned by the context manager. Otherwise None is returned by the context
    manager. The objects appended to the list are arguments whose attributes
    mirror the arguments to showwarning().

    The 'module' argument is to specify an alternative module to the module
    named 'warnings' and imported under that name. This argument is only useful
    when testing the warnings module itself.

    """

    def __init__(self, *, record=False, module=None):
        """Specify whether to record warnings and if an alternative module
        should be used other than sys.modules['warnings'].

        For compatibility with Python 3.0, please consider all arguments to be
        keyword-only.

        """
        self._record = record
        self._module = sys.modules['warnings'] if module is None else module
        self._entered = False

When I try:

with warnings.catch_warnings(category=FutureWarning, record=True):

I get this error:

>>> from determined.experimental import Checkpoint
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/taylor/git/determined/harness/determined/experimental/__init__.py", line 3, in <module>
    with warnings.catch_warnings(category=FutureWarning, record=True):
TypeError: __init__() got an unexpected keyword argument 'category'

I like the idea but I'm not seeing how it could actually be used

[wes-turner]

Aw, dang.

Changed in version 3.11: Added the action, category, lineno, and append parameters.

[wes-turner]

It's all hard for me to conceptualize, but does stacklevel=2 localize the warning better (to the importer, rather than to the grand-importer)?

[tayritenour]

2 might be better, I think I left it at 3 from a previous iteration of where I had this warning. I had it actually inside the files themselves.

From a raw python shell:

>>> from determined.common.experimental.checkpoint import Checkpoint
<stdin>:1: FutureWarning: The 'determined.common.experimental' module is deprecated and will be removed in future versions. Please import from 'determined.experimental' instead. Example: `from determined.experimental import Determined`

From a python file:

python3 a.py
a.py:1: FutureWarning: The 'determined.common.experimental' module is deprecated and will be removed in future versions. Please import from 'determined.experimental' instead. Example: `from determined.experimental import Determined`
  from determined.common.experimental.checkpoint import Checkpoint

[wes-turner]

LGTM

Could you please add a short description / test plan before merging?