ppannuto / python-titlecase

Python library to capitalize strings as specified by the New York Times Manual of Style
MIT License
249 stars 38 forks source link

Look for the default abbreviations file in titlecase() only when called from command line #59

Closed igorburago closed 4 years ago

igorburago commented 4 years ago

If the titlecase() function is called directly as a library routine, no file system operations should be incurred, unless an effort is made on the caller’s side in the form of passing a filename via the wordlist_file argument.

Fixes #58. May be closed in favor of #60.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-2.3%) to 82.828% when pulling 544f9e1cb131933f38c1c039bb33f283c91afc05 on iburago:master into c88fb220916e926e22d6764f31c78b66387b21f2 on ppannuto:master.

1kastner commented 4 years ago

Do I read correctly that this is more or less a compromise, the version that requires less change but achieves the clarity in code you require?

igorburago commented 4 years ago

Yes, this PR is a compromise: It keeps the abbreviations list building from file within titlecase(), but makes it so that, at least, no file system operations happen by default, unless the caller makes an effort by passing a non-None value to the writelist_file argument.

If #60 could be merged instead, I would prefer that more, because this version (as the current state of the repository) mixes up concerns in the titlecase() function, dilutes its API, and makes it less useful as a library function (which is one of the great advantages of this little package—that it is not just the command-line tool).

ppannuto commented 4 years ago

I'm pretty convinced by the arguments in #60, closing this in favor.