LSSTDESC / Monitor

Extract light curves for time-variable cosmological objects
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

PEP8 style #14

Closed drphilmarshall closed 8 years ago

drphilmarshall commented 8 years ago

For my own edification, I tried to make monitor.py pass my pylint linter. See teh "Changed Files": this is about as far as I could get. The main thing was that we had mixedCase variables everywhere - is that a DM standard?

rbiswas4 commented 8 years ago

There is a standard that attribute names should be CamelCase with a leading lower case. But I am not sure about temporary variables.

http://developer.lsst.io/en/latest/coding/python_style_guide.html#naming-conventions

brianv0 commented 8 years ago

It's in the original style guide, but it's almost effectively deprecated:

https://jira.lsstcorp.org/browse/RFC-81 https://jira.lsstcorp.org/browse/RFC-97

Since C++ doesn't enforce a de-facto style the same way Python (or java) does, it was agreed that effectively adopting PEP8 for C++ stack projects (using underscores for functions_and_variables, CamelCase for classes) is the way forward, but it wasn't decided on exactly how that should happen. Because the amount of legacy code, they want to approach this change with "baby steps".

Note: PEP8 allows camelCase methods and variables but it encourages underscores.

drphilmarshall commented 8 years ago

OK, thanks! I vote we merge this PR but I leave it to you, @jbkalmbach My main motivation is to get my linter to shut up :-)

On Wed, Mar 2, 2016 at 1:58 PM, Brian Van Klaveren <notifications@github.com

wrote:

It's in the original style guide, but it's almost effectively deprecated:

https://jira.lsstcorp.org/browse/RFC-81 https://jira.lsstcorp.org/browse/RFC-97

Since C++ doesn't enforce a de-facto style the same way Python (or java), it was agreed that effectively adopting PEP8 for C++ stack projects (using underscores for functions and variables, CamelCase for classes) is the way forward, but it wasn't decided on exactly how that should happen. Because the amount of legacy code, they want to approach this change with "baby steps".

Note: PEP8 allows camelCase methods and variables but it encourages underscores.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/Monitor/pull/14#issuecomment-191451990 .

jchiang87 commented 8 years ago

Hey @drphilmarshall,

You can silence warnings selectively via your .pylintrc file:

$ cat ~/.pylintrc
[MESSAGES CONTROL]
disable=C0103

Depending on your version of pylint, it might be disable_msgs instead. See http://pylint-messages.wikidot.com/all-codes

drphilmarshall commented 8 years ago

Aha! This is where we would write a DESC pylint configuration, perhaps in an "Environment" module...

On Wed, Mar 2, 2016 at 2:46 PM, James Chiang notifications@github.com wrote:

Hey @drphilmarshall https://github.com/drphilmarshall,

You can silence warnings selectively via your .pylintrc file:

$ cat ~/.pylintrc [MESSAGES CONTROL] disable=C0103

Depending on your version of pylint, it might be disable_msgs instead. See http://pylint-messages.wikidot.com/all-codes

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/Monitor/pull/14#issuecomment-191476771 .

jbkalmbach commented 8 years ago

I agree Phil that we should try to use linters to make our code better! Merged!

rbiswas4 commented 8 years ago

There is also a pep8 converter:

https://pypi.python.org/pypi/autopep8

that converts to pep8 style code that does not conform to it.

On Wed, Mar 2, 2016 at 4:16 PM, Phil Marshall notifications@github.com wrote:

Aha! This is where we would write a DESC pylint configuration, perhaps in an "Environment" module...

On Wed, Mar 2, 2016 at 2:46 PM, James Chiang notifications@github.com wrote:

Hey @drphilmarshall https://github.com/drphilmarshall,

You can silence warnings selectively via your .pylintrc file:

$ cat ~/.pylintrc [MESSAGES CONTROL] disable=C0103

Depending on your version of pylint, it might be disable_msgs instead. See http://pylint-messages.wikidot.com/all-codes

— Reply to this email directly or view it on GitHub < https://github.com/DarkEnergyScienceCollaboration/Monitor/pull/14#issuecomment-191476771

.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/Monitor/pull/14#issuecomment-191504380 .