Read from an abbreviations file outside of titlecase() and only when invoked from command line#60
Conversation
|
If we choose this PR over #59, we might want to rename But we can even keep that function private, of course, unless we Here I keep this function importable, but not added to |
|
As a side note, I am not sure it is a great idea to have that I would remove that logging call or, at least, would protect it I am also not sure that we really need any logging in |
|
I don't believe that this library targets high-performance use cases. Moreover, one old programmer's wisdom: Only optimize if you need to optimize! As long as there is no urge, it might be a waste of time and it might clutter the code. I could be persuaded by proper benchmarks though. In addition, have you checked the logging library whether they maybe already have something similar in place? I guess they did optimize their code. |
|
To be honest, for my use case I only care about the command line interface so the internals do not bother me so much as long as nothing breaks, there are tests, and everything is nicely documented. |
|
Regarding logging: If Python is started with the The logger’s It is unfortunate that no benchmarks were made when this logging I do not have time to do benchmarks at the moment, so, as I said (I brought up logging as a side node, just out of curiosity.) |
|
As to the subject of this PR, your interest is not affected, then, since:
|
|
Although I have touched on the advantages of this PR in #58, let me
|
|
That sounds great to me! |
If the titlecase() function is called directly (say, from a library), the caller can still use a custom list of abbreviations by passing an appropriate function as callback.
|
This makes a lot of sense to me. I was probably a little quick to move on pushing out the prior iteration of this, it had just sat for a while so I felt bad. Strictly speaking, I think this will also require a 2.0 release as it changes the interface; not that I think a ton of folks have pulled the 1.0 version yet, but I would like to try to be a good citizen of the semver universe. |
|
Changing the major version number due to the change of the API However, unless you have to, could you please postpone making |
|
No urgency to release here. Excited to see the new PRs! |
Major (interface) changes (#60) -- big thanks @iburago!!: - Read from an abbreviations file only when invoked from command line - Rename and refactor the create_wordlist_filter() function Minor changes: - #63: Do not capitalize small words occurring within hyphenated word groups (thanks @iburago!) - #65: Always capitalize 'Mc'-prefixed small words in compound word groups (thanks @iburago!) - #67: Don't capitalize Mr, Ms, Mrs (thanks @GurraB!) - #71: Add Manifest.in and include license file in src dist (thanks @synapticarbors!)
If the
titlecase()function is called directly (say, from a library),the caller can still use a custom list of abbreviations by passing
an appropriate function as
callback.Fixes #58.