Page 1 of 3

Coding Style

Posted: Wed Jul 11, 2012 1:56 am
by satoon101
One thing I learned while working with the GunGame5.1 dev team is that it is very important to all follow the same coding style. This thread is basically an outline of what I believe our style (Python-side) should be for all modules/APIs.

I am certain that most of you have heard of PEP 8, and some of you probably already adhere to at least part of its proposal. I would like us to follow this PEP as closely as possible. I would also say that PEP 257, which deals with documenting your code properly, is something that is not only recommended, but required. Some main points:

  • Naming conventions:
    • Class names should use CamelCase

      Syntax: Select all

      class TestClass(object):
      pass
    • No underscores in class names except for a leading underscore to mark as "private"

      Syntax: Select all

      class _PrivateTestClass(object):
      pass
    • Constants must be ALL_CAPS with an underscore separating the words

      Syntax: Select all

      CONSTANT_VALUE = True
    • Class attributes/properties/methods should be lower_case_with_underscores

      Syntax: Select all

      class TestClass(object):
      def __init__(self, value1, value2):
      self.value1 = value1
      self._value2 = value2

      @property
      def value2(self):
      return self._value2

      def some_method(self):
      return self.value1 + self.value2
    • Singleton objects or "the instance" objects (objects that should be the one and only instance of a class) should also use lower_case_with_underscores

      Syntax: Select all

      class TestClass(object):
      pass

      test_class = TestClass()
    • All global variable objects (which are not constants) should also use lower_case_with_underscores

      Syntax: Select all

      start_value = True
    • Any global variables created in the scope which are not to be imported by other modules should use a leading underscore

      Syntax: Select all

      _private_variable = False
    • Only built-in magic methods should have 2 leading and trailing underscores
    • ALL names should be very descriptive as to what the object does or is

  • Formatting:
    • Lines must be no longer than 80 characters (that includes the built-in \n, making it 79 characters of code)
    • MUST use 4 space indentation, no TABs
    • MUST have 2 blank lines prior to functions and classes
        If the function/class is starting a new <Section> (explained below), the blank lines need to be prior to the section separator
    • All other lines should only have 1 blank line between
        An exception that is sometimes used is 2 blank lines prior to a <Section> change (explained below)
    • No extra whitespace at the end of a line
        This also means that any "blank" lines should have absolutely nothing, not even spaces, in them
    • Exactly 1 space between arguments (after the comma) for all functions/classes/methods, including when calling them
      • No spaces at all when only 1 argument
      • No leading space before first argument or trailing space after last argument
    • Exactly one space after a : when setting an item in a dictionary
    • There must be exactly one blank line at the end of the file

  • Documentation:
    • All classes/methods/properties/functions must be followed by a comment using a triple quote docstring (either apostrophes or quotes)
    • Most lines of code should follow a one line comment (using the # character)
        Some exceptions apply, like comments that suffice for multiple lines
    • Files should always start with the line:

      Syntax: Select all

      # ../<path from source-python directory>
    • Different sections of code should be separated by the following lines (where <Section> should describe what is in the following section):

      Syntax: Select all

      # =============================================================================
      # >> <Section>
      # =============================================================================
      • Some examples of what <Section> types there are to be used:
        • IMPORTS
        • GLOBAL VARIABLES
        • CLASSES
        • FUNCTIONS

  • Import comments:
    • Separate base Python imports (including site-packages) from Source.Python core imports with 1 blank line
    • Use a comment line with 1 space after the # showing whether the current section is from Python or Source.Python
    • Python imports should always go first, then Source.Python imports
    • Separate each module by using a comment about which module is being imported (with 3 spaces after the # before the modules name)
    • Modules should be listed in alphabetic order
    • Core Source.Python modules (from C++, imported from Source) should be first among the Source.Python imports and should not have a comment
      • Anything directly imported from the _libs directory (not one of its sub-directories) should follow the Core module imports and behave similarly
      • Currently this only includes the "paths" module
    • Example:

      Syntax: Select all

      # =============================================================================
      # >> IMPORTS
      # =============================================================================
      # Python Imports
      # Configobj
      from configobj import ConfigObj
      # OS
      from os.path import dirname
      from os.path import join
      # Traceback
      from traceback import format_exception

      # Source.Python Imports
      from Source import Engine
      from Source import Shared
      from paths import GAME_PATH
      # Addons
      from addons.manager import AddonManager
      # Core
      from core.decorators import BaseDecorator
      # Events
      from events.manager import EventRegistry
      from events.decorator import Event

  • Do's and Don'ts of Importing (the use of "Good" and "Bad" just show what is/isn't acceptable for "this" project. other people prefer to use the opposite, which is perfectly fine):
    • Never import "all" from a module:

      Syntax: Select all

      # Bad
      from something import *

      # Good
      from something import one_object
      from something import second_object
    • For "most" imports, import each object individually, and on a separate line:

      Syntax: Select all

      # Bad
      from os.path import dirname, join, curdir

      # Good
      from os.path import dirname
      from os.path import join
      from os.path import curdir

      # "Ok", but use only when necessary
      import os.path
      import sys

Most items under the Formatting section should be caught by the PEP8 checker:
http://pypi.python.org/pypi/pep8/

If anyone has any other proposals/issues/updates for this, please feel free to share your thoughts.

Satoon

Posted: Wed Jul 11, 2012 6:20 am
by Ayuto
I agree with most of that. Personally, I just don't like function/method names in lower case with underscores, but i guess I will try to use it :D

But this would mean that Source.Python's function/method names need to be renamed, because they use the CapitalizedWords stlye.

Posted: Wed Jul 11, 2012 6:47 am
by Omega_K2
I agree, but I don't like underscore function names. Also when SP more or less directly exposes the EngineAPI it seems to make sense to actually keep the CamelCase functions as they resemble the way the game is written (instead of forcing them into a PEP8 style).

Posted: Wed Jul 11, 2012 3:39 pm
by your-name-here
Ayuto wrote:I agree with most of that. Personally, I just don't like function/method names in lower case with underscores, but i guess I will try to use it :D

But this would mean that Source.Python's function/method names need to be renamed, because they use the CapitalizedWords stlye.


That's because I directly wrap them using C++ macros. They're named however VALVe decides to name them. It would end up being a lot of extra work to change them to camel case. Check out wrap_macros.h, you'll see the macros I am using.

Posted: Thu Jul 12, 2012 12:26 am
by satoon101
Again, this is for the "Python" API. Those differences will help to differentiate between whether code is based in the Python or C++ side.

Also, if you look into Python3.3 modules (or even 2.5.2 which ES uses), you will notice most of them also use this naming style.

Satoon

Posted: Thu Jul 12, 2012 9:05 pm
by Monday
Omega_K2 wrote:I agree, but I don't like underscore function names. Also when SP more or less directly exposes the EngineAPI it seems to make sense to actually keep the CamelCase functions as they resemble the way the game is written (instead of forcing them into a PEP8 style).


CamelCase is more of a java standard, view the page on PEP8 http://www.python.org/dev/peps/pep-0008/

Posted: Thu Jul 12, 2012 11:14 pm
by Omega_K2
satoon101 wrote:Again, this is for the "Python" API. Those differences will help to differentiate between whether code is based in the Python or C++ side.

Also, if you look into Python3.3 modules (or even 2.5.2 which ES uses), you will notice most of them also use this naming style.

Satoon


Yes, but this kind of inconsistency is even worse. I renember reading somewhere something like:
Consistency within a language is important.
Consistency within a project is more important.
Consistency within a module is most important.

Following that philosophy, it should at least be consistant in the project and if consistency with the CAPI is to be kept, CamelCase makes sense to use instead of the underscore_function_names style.

Monday wrote:CamelCase is more of a java standard, view the page on PEP8 http://www.python.org/dev/peps/pep-0008/


Did you even read what I said?

Posted: Fri Jul 13, 2012 12:18 am
by Monday
Omega_K2 wrote:Yes, but this kind of inconsistency is even worse. I renember reading somewhere something like:
Consistency within a language is important.
Consistency within a project is more important.
Consistency within a module is most important.

Following that philosophy, it should at least be consistant in the project and if consistency with the CAPI is to be kept, CamelCase makes sense to use instead of the underscore_function_names style.



Did you even read what I said?



Of course I did.. I just prefer following the PEP8 standard for the python language, but perhaps I am biased as we have been using it in gungame for quite some time. The consistency argument makes sense here as well, so I suppose we can do the logical thing here and figure out what is best for the system as a whole.

In the end it really does not matter what we end up using... I prefer the usage of underscores because that is what I have been doing for years, and honestly it won't bother me in the slightest to use CamelCase instead. I do like adhering to the language itself though... PEP8 is what most of the built in python functionality follows.

Posted: Fri Jul 13, 2012 10:06 pm
by Omega_K2
Monday wrote:Of course I did.. I just prefer following the PEP8 standard for the python language, but perhaps I am biased as we have been using it in gungame for quite some time. The consistency argument makes sense here as well, so I suppose we can do the logical thing here and figure out what is best for the system as a whole.

In the end it really does not matter what we end up using... I prefer the usage of underscores because that is what I have been doing for years, and honestly it won't bother me in the slightest to use CamelCase instead. I do like adhering to the language itself though... PEP8 is what most of the built in python functionality follows.


Ah well, I was under the impression you didn't really it carefully at least. Though so things are clear for everyone, the thing is the only problem with PEP8 is the way how the function names are typed either in all_lower_with_underscore or CamelCase. Just for everyone, I'll cite the respective passage of PEP8 I renembered ;)

PEP8 wrote:A Foolish Consistency is the Hobgoblin of Little Minds

One of Guido's key insights is that code is read much more often than it is written. The guidelines provided here are intended to improve the readability of code and make it consistent across the wide spectrum of Python code. As PEP 20 says, "Readability counts".

A style guide is about consistency. Consistency with this style guide is important. Consistency within a project is more important. Consistency within one module or function is most important.

But most importantly: know when to be inconsistent -- sometimes the style guide just doesn't apply. When in doubt, use your best judgment. Look at other examples and decide what looks best. And don't hesitate to ask!

Two good reasons to break a particular rule:

When applying the rule would make the code less readable, even for someone who is used to reading code that follows the rules.
To be consistent with surrounding code that also breaks it (maybe for historic reasons) -- although this is also an opportunity to clean up someone else's mess (in true XP style).


Especially the high lightend (bold) part is interesting - logically this leads me to 3 approaches we can do:

1. Use lower_and_underscore functions
2. Use lower_and_underscore functions and matching CamelCase (or whatever) for Engine functions
3. Use CamelCase for every function

1) Pro: Consistent with python
Contra: Inconsistent with Engine API & requires rewriting of the wrapping macros
2) Pro: py stuff is consistent with python, Engine API style is kept
Contra: pretty messy as a project (may be very weird especially with extra functions ex. Engine: GetThis Python extension of API: get_this_advanced)
3) Pro: Consistent with the EngineAPI
Contra: Inconsistent with the python language standart (PEP8 style proposal)

As for rating these options I think 2 is cleary the worst, because it is a complete mess up. 1 is ok, but would require rewriting of every marco and if anyone has experience with the engine api he'll have to guess the python name. 3 is the best solution imho, because it is only inconsistent with python, but since it is consistent thoughout the entire SP plugin and its modules, it should not hard to differiente between python and SP (if you are used to python, you know the py stuff already - SP is new and since the style is consistent, it should also be easy to pickup :P)

Posted: Fri Jul 13, 2012 10:31 pm
by satoon101
Well, 2 things to look at here.

First, we have Python, which most of is consistent in using lower_and_underscore. For the Python-side coders, keeping that consistency is key.

Then, we have Valve's code (which is just being wrapped by the plugin) that is CamelCase. For the C++ coders, keeping that consistency is key.

While this may be 1 project, there are 2 separate groups working on it. As long as those 2 groups know the consistency between each other (which those of us who have push capabilities do), it is mostly then about the scripters. As long as we have proper documentation, they should be fine.

Calling this style a "complete mess up" is being highly over-dramatic...

Satoon

Posted: Sat Jul 14, 2012 6:13 am
by iiya
While this may be 1 project, there are 2 separate groups working on it. As long as those 2 groups know the consistency between each other (which those of us who have push capabilities do), it is mostly then about the scripters. As long as we have proper documentation, they should be fine.

[color="#cc0066"]but i think it will be beneficial for the scripters to have an API whose function naming is consistant with engine functions since i'm sure many scripters use the game/engine sdks for reference (especially if they are going to make use of the dynamic hooking features, etc). i guess it also depends on how abstract the Python API will be though.[/color]

Posted: Sun Jul 15, 2012 11:18 pm
by BackRaw
I'd stick with either mixedCase (they way, the ES2 libs are written) or CamelCase, because if you talk about simplicity here, and in this thread you talk about using uderscore_function_names: since the Valve SDK is written CamelCase, SP is CamelCase, so why shouldn't Python be?

1 project, 1 coding style, 1 documentation style, and so on ;)

Posted: Mon Jul 16, 2012 12:13 am
by satoon101
Ok, since everyone else seems like they want this change, I'll work on updating the current version with these changes. I'll update the main post above when I get the chance. To be consistent, CamelCase will be used in the future.

Satoon

Posted: Mon Jul 16, 2012 1:05 am
by satoon101
Updated main post. If anything is missing (which I am sure there is plenty missing), please feel free to mention it here.

Satoon

Posted: Mon Jul 16, 2012 5:39 am
by BackRaw
Thanks :) nice to see this being taken very seriously.

Posted: Mon Jul 16, 2012 11:50 pm
by satoon101
Updated main post with some more points.

Satoon

Posted: Tue Aug 07, 2012 7:58 am
by satoon101
Updated main post with 2 new sections ("Import comments" and "Do's and Don'ts of Importing").

Satoon

Posted: Thu Aug 16, 2012 2:48 pm
by Tuck
Quick question i've tried a half hour now to get the syntax checker for PEP8 to work but, can't seem to figure it out >_>

http://pypi.python.org/pypi/pep8/#downloads

downloaded it and unpacked dont know where to execute the commands tried to run the pep8.py from the dir but in idle it errors with
Traceback (most recent call last):
File "C:\Users\Tuck\Downloads\pep8-1.3.3\pep8.py", line 1054, in <module>
stdin_get_value = TextIOWrapper(sys.stdin.buffer, errors='ignore').read
AttributeError: buffer
>>>

Posted: Thu Aug 16, 2012 6:23 pm
by BackRaw
Hhhmmm... I find these ones veeeeeeeeery ugly:

Syntax: Select all

# Python Imports
# OS
from os.path import dirname
from os.path import join

# Source.Python Imports
...
Why not just leave it to that?

Syntax: Select all

# Python Imports
from os.path import dirname
from os.path import join

# Source.Python Imports
...
Looks easier to read IMRHO... AAAAAAAAAND, secondly, these

Syntax: Select all

#   OS
and stuff really DON'T explain anything...

Posted: Fri Aug 17, 2012 12:22 am
by satoon101
Tuck wrote:Quick question i've tried a half hour now to get the syntax checker for PEP8 to work but, can't seem to figure it out >_>

http://pypi.python.org/pypi/pep8/#downloads

downloaded it and unpacked dont know where to execute the commands tried to run the pep8.py from the dir but in idle it errors with
That will happen if you try to import it in IDLE. Now that you attempted to import it, it should have created a .pyc file for it. Whatever folder you installed the pep8.py file into, there should now be a __pycache__ folder. Go into that folder, and you should see a file with pep8 in the name, with the extension .pyc.

What I typically do is take the .pyc file, and put it in its own directory (I use D:\pep8\). Once you have it in the directory you wish to have it in, open your command console and navigate to that folder. Then, use the following in the console (this is using my pep8 directory as well as the location of my server's addons):

Code: Select all

C:\Windows\system32>d:

D:\>cd pep8

D:\pep8>pep8.pyc d:/servers/csgo/csgo/addons/source-python/_libs
Satoon