Skip to content

Black code formatter #82

Open
Open

Description

@gb0101010101

I setup black in virtualEnv and everything appears to be working. It is however changing source code dramatically. e.g.

actions.py

Before

'''
This class does most of the heavy lifting in processing messages from the UI client.
'''
class Actions(MakesmithInitFuncs):

    Device.pin_factory = MockFactory()

    def processAction(self, msg):
        '''
        When a message comes in via the UI client it gets parsed and processed here.
        :param msg: json of message from UI client
        :return:
        '''
        try:
            # Commands allowed during sending gcode.  These commands won't screw something up.
            if msg["data"]["command"] == "createDirectory":
                if not self.createDirectory(msg["data"]["arg"]):
                    self.data.ui_queue1.put("Alert", "Alert", "Error with creating directory.")
            elif msg["data"]["command"] == "stopZ":
                if not self.stopZ():
                    self.data.ui_queue1.put("Alert", "Alert", "Error with stopping Z-Axis movement")
            elif msg["data"]["command"] == "stopRun":

After

"""
This class does most of the heavy lifting in processing messages from the UI client.
"""


class Actions(MakesmithInitFuncs):

    Device.pin_factory = MockFactory()

    def processAction(self, msg):
        """
        When a message comes in via the UI client it gets parsed and processed here.
        :param msg: json of message from UI client
        :return:
        """
        try:
            # Commands allowed during sending gcode.  These commands won't screw something up.
            if msg["data"]["command"] == "createDirectory":
                if not self.createDirectory(msg["data"]["arg"]):
                    self.data.ui_queue1.put(
                        "Alert", "Alert", "Error with creating directory."
                    )
            elif msg["data"]["command"] == "stopZ":
                if not self.stopZ():
                    self.data.ui_queue1.put(
                        "Alert", "Alert", "Error with stopping Z-Axis movement"
                    )
            elif msg["data"]["command"] == "stopRun":

Do I have to do something to configure black or is the source code in repo just not formatted using it?
If the latter then how should we go about doing a source code formatting change this big?

Activity

  1. davidelang commented on Jan 23, 2020

    @davidelang
  2. gb0101010101 commented on Jan 23, 2020

    @gb0101010101
    ContributorAuthor

    Read up on black and see that it supports using a pyproject.toml for configuration. Here is one I rolled.

    pyproject.toml

    [tool.black]
    line-length = 120
    skip-string-normalization = true
    target-version = ['py36', 'py37', 'py38']
    include = '\.pyi?$'
    exclude = '''
    /(
        \.eggs
      | \.git
      | \.hg
      | \.mypy_cache
      | \.tox
      | \.venv
      | _build
      | buck-out
      | build
      | dist
      # The following are specific to Black, you probably don't want those.
      | blib2to3
      | tests/data
      | profiling
    )/
    '''
    

    skip-string-normalization = true stops it from replacing all single quotes with double quotes.
    line-length = 120 set max line length. Don't know what sane value this should use. The default is 88 and in other languages the max I have seen is 90.

    After toml

    '''
    This class does most of the heavy lifting in processing messages from the UI client.
    '''
    
    
    class Actions(MakesmithInitFuncs):
    
        Device.pin_factory = MockFactory()
    
        def processAction(self, msg):
            '''
            When a message comes in via the UI client it gets parsed and processed here.
            :param msg: json of message from UI client
            :return:
            '''
            try:
                # Commands allowed during sending gcode.  These commands won't screw something up.
                if msg["data"]["command"] == "createDirectory":
                    if not self.createDirectory(msg["data"]["arg"]):
                        self.data.ui_queue1.put("Alert", "Alert", "Error with creating directory.")
                elif msg["data"]["command"] == "stopZ":
                    if not self.stopZ():
                        self.data.ui_queue1.put("Alert", "Alert", "Error with stopping Z-Axis movement")
                elif msg["data"]["command"] == "stopRun":
                    if not self.stopRun():
                        self.data.ui_queue1.put("Alert", "Alert", "Error with stopping run")
                elif msg["data"]["command"] == "pauseRun":
                    if not self.pauseRun():
                        self.data.ui_queue1.put("Alert", "Alert", "Error with pausing run")
                elif msg["data"]["command"] == "resumeRun":
    

    Should we include the toml file in the project. Do you have an existing one that you are using.

  3. davidelang commented on Jan 23, 2020

    @davidelang
  4. gb0101010101 commented on Jan 23, 2020

    @gb0101010101
    ContributorAuthor

    I am fine with 120 line length as most IDEs provide enough space in the editor window when using default layouts. But I'm a complete noob to Python.

    I have read that Python Devs are VERY picky about code format. e.g. Using double quotes instead of single. black makes it difficult to override many setting purely to force the same coding style. Looking at the code in Python Libraries; all of it seems to be using stock black settings as show in OP After sample.

    I generally just use vi for my programming

    There is a black plugin for vim

    I would suggest that you make one commit that reformatted all code using this
    tool

    Yes completely agree. I would rather not do it and have it come straight from this project using a unique version number in case things go horribly wrong.

    Here's a test branch with changes using stock black setting just updating Actions/*.py
    (1,704 additions and 760 deletions.)
    gb0101010101/WebControl@master...gb0101010101:gb0101010101-test-black-stock

    And here's another using the toml settings. (681 additions and 609 deletions.)
    gb0101010101/WebControl@master...gb0101010101:gb0101010101-test-black-toml

    Still masses of changes.

  5. madgrizzle commented on Jan 23, 2020

    @madgrizzle
    Collaborator

    So johnboiles recommended using black and I did use it for a while, but it bothered me with all the line breaks and I really haven't used it since.

  6. gb0101010101 commented on Jan 27, 2020

    @gb0101010101
    ContributorAuthor

    Well I am all for a defined coding format. Black seems to format they way most Python projects do. It may be annoying at first but it will greatly help in the long run.

    Since we only have a few people committing changes right now it seem like a good time to format the code. I can do it and test if you want.

  7. madgrizzle commented on Jan 27, 2020

    @madgrizzle
    Collaborator

    One problem I have is that I code on multiple machines.. sometimes on an RPI. some times on my desktop, sometimes on my laptop. I think only that laptop has black.. so I'd need to figure things out for the other two (i.e., get black installed) before I could keep up with it.

  8. davidelang commented on Jan 27, 2020

    @davidelang
  9. gb0101010101 commented on Jan 28, 2020

    @gb0101010101
    ContributorAuthor

    @madgrizzle One problem I have is that I code on multiple machines

    Black is a Python plugin so runs on everything that Python does. On Rpi just do
    pip install black
    But real question is why are your coding on RPi? Are you using an IDE or CLI editor? Some CLI editors like VIM also support Black (mentioned in previous comment)

    @davidelang code that isn't formatted correctly may not be working as intended

    Right, This could cause problems when doing a large scale code format commit. This is what I was offering to test. Though the testing I was thinking of is for code errors rather than functionality.

  10. madgrizzle commented on Jan 28, 2020

    @madgrizzle
    Collaborator

    I have a controller hooked to my rpi and sometimes I just use notepad++ to edit files via ssh because I can't hook a controller up to the machine I'm using at the moment.

    I'm not against black.. just haven't gotten all setup for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions