r1chardj0n3s / parse

Parse strings using a specification based on the Python format() syntax.
http://pypi.python.org/pypi/parse
MIT License
1.72k stars 101 forks source link

Don't use `import *` to introduce the module #104

Closed reynoldsnlp closed 3 years ago

reynoldsnlp commented 4 years ago

Awesome package! Using wildcard imports (import *) obfuscates which parts of the example code come from the module, and it violates the suggestions in PEP 8. The fact that your implementation of __all__ is motivated by name conflicts, and not API design, is exactly why wildcard imports are usually discouraged. I imported the module as pe, partially because p was already taken in your examples, and partially because it looks like re ;-p.

r1chardj0n3s commented 4 years ago

Thanks so much for the submission, but I'm a -1 on this patch. While adding an initial example showing that regular import parse; ... might be beneficial, I personally prefer the simpler usage that import * provides, given that it deliberately (through all) only imports 4 symbols. I say this even as someone who thinks PEP 8 is a great thing - sometimes practicality beats purity.

reynoldsnlp commented 4 years ago

I'll push back a little, but I won't be offended if you reject the PR! :-)

I thought of the edits, not out of a knee-jerk PEP8 reaction, but because in reading the example code, I felt like it took extra effort to identify which pieces were coming from parse. I think being explicit is more clear in code that is intended to introduce a module for the first time.

As for the number of symbols imported, import parse only imports 1 symbol. ;-)

Whatever you decide is fine with me.

eirnym commented 4 years ago

I'm for the patch as it gives more clarity if you're not using an IDE to say where the dependency come from.