ppannuto / python-titlecase

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

It is impossible to prevent titlecase() from reading the ‘~/.titlecase.txt’ file #58

Closed igorburago closed 4 years ago

igorburago commented 4 years ago

It makes sense to have that behavior as default for the titlecase command (especially when it is invoked directly by the user from the command line), but not for the titlecase() function that may be called from some library code, which should not be accessing the file system at all!

Besides, when writing a script using titlecase(), one often needs to have a way to ensure consistent results across all users’ machines, regardless of what each particular user has in their ~/.titlecase.txt file (and whether they have the file at all). Also, as for libraries, for some scripts it may not even be appropriate (nor allowed) to access the user’s file system.

I propose to change the behavior of the titlecase() function to, at the very least, not attempt reading ~/.titlecase.txt (or any other files, for that matter) when its wordlist_file argument has the value of None. In general, unless an effort is made by the caller, the call to titlecase() should not incur any filesystem operations.

I have made the minimal required fix for that in #59.

igorburago commented 4 years ago

By the way, it is probably better for the quality of the titlecase() API to not support any lists of abbreviations in a separate interface (e.g., even via an extra argument), because the caller can already do that by providing their own function in the callback argument.

This way, the caller can programmatically populate the list of abbreviations in whatever way it makes sense and is convenient for them in the context of their code and handle the matching logic in their custom callback, instead of being tied to creating a file (or using a pseudo-file object).

I have made the corresponding change in #60.

igorburago commented 4 years ago

Neither #60 nor #59 change the current behavior of the command line tool; they only modify the interface of the titlecase() function.

I would personally much prefer #60 over #59, as it allows for more versatility in using the titlecase module as a library, while keeping the titlecase() function API small and clean.

However, at the end of the day, it is, of course, your call as the maintainer, @ppannuto.

igorburago commented 4 years ago

@1kastner: Since you are the original author of the feature, feel free to join the discussion. Specifically, I would be most interested to know whether you will be all right with merging #60 (see above).