smart_open icon indicating copy to clipboard operation
smart_open copied to clipboard

Deprecate smart_open.s3_iter_bucket in favor of smart_open.s3.iter_bucket

Open mpenkov opened this issue 4 years ago • 2 comments

The logger will only print if the root logger is configured which is no guarantee.

As far as that import path in __init__.py goes it would seem that it would need to be removed. Once those underlying dependencies are dropped from being installed I believe the import will cause issues. If sys.exit is dropped and the shortcut path maintained it would cause the import error to be suppressed here and thrown elsewhere. Whereas by maintaining the sys.exit the program exits exactly where the message to fix the issue is printed vs exiting with an import error elsewhere that doesn't immediately tell you what the issue is.

Originally posted by @Amertz08 in https://github.com/RaRe-Technologies/smart_open/pull/454

mpenkov avatar Apr 07 '20 07:04 mpenkov

I think this is fairly straightforward to implement as the function is aliased on import. You can easily provide a deprecation warning.

# s3.py
def s3_iter_bucket(*args, **kwargs):
    warnings.warn("deprecation msg", DeprecationWarning)
    yield iter_bucket(*args, **kwargs)

This would make it so the warning is only thrown when using the deprecated name and not the actual name. You'd have to modify the import path in __init__.py

Probably should use the actual function interface. I just used *args, **kwargs for brevity.

Amertz08 avatar Apr 07 '20 19:04 Amertz08

That, and you can also perform the import inside the actual function instead at module scope.

mpenkov avatar Apr 08 '20 00:04 mpenkov