seanfisk / python-project-template

A template Python project with a focus on best practices.
Other
543 stars 168 forks source link

Use encoding comments consistently in `*.{py,.py.tpl}` files #30

Closed Zearin closed 10 years ago

seanfisk commented 10 years ago

My current practice was to only put encoding declarations in files that used Unicode characters. Do you think that it's a better practice to put them in every file?

Zearin commented 10 years ago

My current practice was to only put encoding declarations in files that used Unicode characters. Do you think that it's a better practice to put them in every file?

Well, it is much simpler. (No checking, “Let’s see…are there unicode characters in this file?”)

Also, some code editors use it as a hint.

In the end, it’s a matter of taste. Up to you. :)

seanfisk commented 10 years ago

Well, it is much simpler. (No checking, “Let’s see…are there unicode characters in this file?”)

I've done this before, and I admit it's annoying. It's also annoying to type in the declaration for each file, but ideally you would have some sort of helper that creates a new Python module with the necessary boilerplate. I don't have that, but that's certainly no excuse to avoid creating one :smiley:

I want to merge this, but I also don't want to forget to add it to each of my files. Ideally, we would have a testing tool (flake8 / pylint) which checks for this and yells at you if you don't have it. Do you know off-hand if either can do this?

By the way, for when this gets merged, are you aware of the PPT updater tool? It's not documented, but it can update your PPT-based project to a newer version of the PPT by incorporating upstream changes to PPT into your project. The reason I mention it is because I'm looking for others to test it out (and it's useful!).

Zearin commented 10 years ago

I want to merge this, but I also don't want to forget to add it to each of my files. Ideally, we would have a testing tool (flake8 / pylint) which checks for this and yells at you if you don't have it. Do you know off-hand if either can do this?

No, but one solution would simply be to include a test in your boilerplate that looks for the comment in the first X lines of each file. (I believe it’s always supposed to be the first line, UNLESS there’s a shebang, in which case it’s the second line. I’d consider going 1–3 lines further, just to be thorough.)

Alternatively, some editors allow the user to specify a template for new Python files, so they could just do it that way. But that’s outside the scope of the project template, of course.

By the way, for when this gets merged, are you aware of the PPT updater tool?

PPT…?

Oh! I’m guessing you mean “Python Project Template” and not “PowerPoinT”. :tongue: Didn’t put that together until just now.

It's not documented, but it can update your PPT-based project to a newer version of the PPT by incorporating upstream changes to PPT into your project. The reason I mention it is because I'm looking for others to test it out (and it's useful!).

Well, that sounds like the equivalent of a git rebase to me. Except it’s rebasing everything after the initial template setup. I think it’s a good idea, but it might open up “maintenance creep” if there are enough edge cases. If you think that’s unlikely, or you’re willing to take it on, then totally go for it!

seanfisk commented 10 years ago

Sorry, I just use PPT out of habit because "Python Project Template" is too long to type :smile:

Alternatively, some editors allow the user to specify a template for new Python files, so they could just do it that way. But that’s outside the scope of the project template, of course.

Possibly not. A template could always be included with the generated project.

Well, that sounds like the equivalent of a git rebase to me. Except it’s rebasing everything after the initial template setup.

Yeah, basically. The difference between git rebase and the updater is that the updater is doing a 3-way merge using generated code. It uses diff3 to accomplish this task.

I think it’s a good idea, but it might open up “maintenance creep” if there are enough edge cases.

I wasn't sure how it was going to work at first. However, I used it on some projects that were far, far behind and it worked surprisingly well. You won't escape without merge conflicts every time, but most conflicts are pretty easy to fix.

I'm still on the fence about this pull request in general. I did some searching and it appears there's no Python convention or suggestion to use UTF-8 as the default source encoding of your source code. One thing in support of it would be this guideline in the Ruby Style Guide, but I haven't seen a similar one for Python.

It's a careful balance — as I see it, either we add the coding comment and test for it, or we don't do it at all. I would hedge a guess that most people don't use Unicode in their Python code. For those people, they may view adding the declaration as a burden (#ifdef ... #define .. #endif anyone?) and choose not to use the PPT because of that. I know that I've dismissed tools on what I call "prescribing personal preference" before.

seanfisk commented 10 years ago

I wish that UTF-8 was just the default encoding for Python source files. But considering it's not, and we want to have the freedom to use Unicode characters, this ultimately makes sense. Merge!