Move contrib.handlers

[leej3]

Moves ignite.contrib.handers to ignite.handlers maintaining backwards compatible. Proposed removal is 0.8.0 0.6.0.

Full test suite was run locally (though with 356skips, mostly due to missing (multi-node, horovod, or xla dependencies). Unrelated failures were observed before and after these changes in ignite/metrics/nlp/test_rouge.py and ignite/handlers/test_checkpoint.py.

[vfdev-5]

@leej3 thanks for the PR!
I think it would be better to split it into multiple ones such that we could review and land them quickier

[leej3]

@vfdev-5 two remaining details that I can spot, and will work through now, are corresponding updates to handlers.rst files, and altering the imports across the codebase (notebooks etc.) to try to guide usage to the new import location of this functionality.

One potential issue is the dependencies that this introduces (I spotted in the guidelines for contributing that the contrib module has "handlers classes that may require additional dependencies". I am assuming that this will not cause an issue but let me know if there is a way of addressing this.

[vfdev-5]

two remaining details that I can spot, and will work through now, are corresponding updates to handlers.rst files, and altering the imports across the codebase (notebooks etc.) to try to guide usage to the new import location of this functionality.

thanks, sounds good!

One potential issue is the dependencies that this introduces (I spotted in the guidelines for contributing that the contrib module has "handlers classes that may require additional dependencies". I am assuming that this will not cause an issue but let me know if there is a way of addressing this.

I think we can remove that sentence where it is written. Overall global idea is to get rid of contrib module (keeping it just for BC).

[leej3]

I'm encountering a circular import with "global_step_from_engine". In ignite.contrib.handlers it is imported in both the top level init and in each logger module e.g. ignite.contrib.handlers.clearml_logger.
As we migrate this functionality we get a circular import because "global_step_from_engine" is defined in the ignite.handlers init.

I think the solution is to remove global_step_from_engine from the namespace of each logger module and change everywhere in the codebase that currently imports it from those modules to

from ignite.handlers import global_step_from_engine

Shall I include a deprecated "global_step_from_engine" in the migrated API for now to help users with the migration? Something like:

# Inside each logger module where global_step_from_engine was available

import warnings

def global_step_from_engine(*args, **kwargs):
    warnings.warn(
        "Importing 'global_step_from_engine' from this module is deprecated and will be removed in a future version. "
        "Please import it from 'ignite.handlers' instead.",
        DeprecationWarning,
        stacklevel=2,
    )
    from ignite.handlers import global_step_from_engine as gsf_engine
    return gsf_engine(*args, **kwargs)

Also, let know if you want/think of another solution.

[leej3]

As discussed, this implementation makes sure that global_step_from_engine is still importable from the logger modules and ignite.handlers (the circular import is solved by defining the function in a utils.py). Another circular import was resolved in ignite/__init__.py. Tests pass for me in ignite.contrib.handlers and ignite.handlers for both commits.

[vfdev-5]

Thanks John, looks good!