Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up keyword arguments #268

Merged
merged 22 commits into from Mar 13, 2019
Merged

Clean up keyword arguments #268

merged 22 commits into from Mar 13, 2019

Conversation

mpenkov
Copy link
Collaborator

@mpenkov mpenkov commented Feb 26, 2019

My second crack at #230

We used to liberally pass arbitrary keyword arguments (e.g. **kwargs). This is a bad practice because it's difficult to keep track of function parameters and ensure they're being passed down to the right function.

This PR starts to clean up our use of arbitrary keyword arguments.

Main changes:

  • renamed smart_open.smart_open to smart_open.open
  • renamed ignore_extension parameter to ignore_ext
  • default open mode is now "r", to match standard library
  • keyword arguments for that function now completely match that of built-in open, with two exceptions (ignore_ext and tkwa)
  • tkwa contains keyword arguments for whatever transport layer smart_open ends up using (e.g. HTTPS, S3, etc)
  • Unsupported keyword arguments don't crash the library (same as old behavior) but cause a log warning (new behavior). Supported keyword arguments are identified via introspection.
  • The transport keyword args are documented both in the top-level open function and each individual module (S3, HTTPS, etc). There is no code duplication because of some docstring magic, hidden from the user.

Advantages:

  • Simpler interface for the user, less arguments to open function
  • Adding additional keyword arguments will no longer require updating the top-level interface
  • Better documentation for keyword parameters (previously, they were documented via examples only)

@mpenkov mpenkov changed the title WIP Clean up keyword arguments Clean up keyword arguments Feb 27, 2019
smart_open/doctools.py Show resolved Hide resolved
@mpenkov mpenkov merged commit 1d608cf into piskvorky:master Mar 13, 2019
@mpenkov mpenkov deleted the so2 branch March 13, 2019 06:09

For internal use only.
"""
import inspect
Copy link
Owner

@piskvorky piskvorky Mar 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style: blank line after (and before) the module docstring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants