piskvorky / smart_open

Utils for streaming large files (S3, HDFS, gzip, bz2...)
MIT License
3.17k stars 383 forks source link

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

Open mpenkov opened 4 years ago

mpenkov commented 4 years ago

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_

Amertz08 commented 4 years ago

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.

mpenkov commented 4 years ago

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