Code Cleanup

[hlorus]

As the addon has grown quite a bit and new contributors are jumping onboard there's some cleanup overdue. This task is used to keep track of code cleanup efforts.

Feel free to suggest additional topics/areas in the comments.

Structure

• [x] #219
• [x] Improve registration flow #223
• [ ] Slip out utilities and constants
• [ ] Split out blender classes into into individual files (PropertyGroups, Operators, Panels, etc...) #229

Style

• [x] #248
• [ ] Add docstrings
• [ ] Add type hints

[bonjorno7]

It helps a lot with maintaining readability if everyone formats their code the same way. I use YAPF for formatting, which can be setup in most popular text editors. Here are my VS Code settings for reference:

{
    "editor.rulers": [
        80,
        120
    ],
    "editor.formatOnPaste": true,
    "editor.formatOnSave": true,
    "python.formatting.provider": "yapf",
    "python.formatting.yapfArgs": [
        "--style",
        "{based_on_style: google, column_limit: 120}"
    ],
    "python.sortImports.args": [
        "-l",
        "120"
    ],
    "[python]": {
        "files.insertFinalNewline": true,
        "files.trimFinalNewlines": true
    },
    "python.linting.enabled": true,
    "python.linting.pylintEnabled": false,
    "files.exclude": {
        "**/*.pyc": true
    }
}

[hlorus]

It helps a lot with maintaining readability if everyone formats their code the same way. I use YAPF for formatting, which can be setup in most popular text editors. Here are my VS Code settings for reference:

{
    "editor.rulers": [
        80,
        120
    ],
    "editor.formatOnPaste": true,
    "editor.formatOnSave": true,
    "python.formatting.provider": "yapf",
    "python.formatting.yapfArgs": [
        "--style",
        "{based_on_style: google, column_limit: 120}"
    ],
    "python.sortImports.args": [
        "-l",
        "120"
    ],
    "[python]": {
        "files.insertFinalNewline": true,
        "files.trimFinalNewlines": true
    },
    "python.linting.enabled": true,
    "python.linting.pylintEnabled": false,
    "files.exclude": {
        "**/*.pyc": true
    }
}

I agree, there should be a formatting rule. I would try to keep it as simple as possible tho so new contributors can easily jump on board without having to deal with alot of configuration first. IMO autopep8 would be a good choice as it's already the default setting in vscode. An alternative could be black where you don't have any configuration at all.

[laundmo]

Another topic to consider is structuring the repository differently, if blender allows. Usually for normal python packages the top level should not include any python source files, instead they should be a src or cad_sketcher directory which contains all source files and a main.py entrypoint.

Along with this i would discourage the use of relative imports, as they can lead to issues with modules on sys.path having the same name and are discouraged by PEP8.

This all hinges on how blender works with addons, a topic which i have no experience with.

[hlorus]

Another topic to consider is structuring the repository differently, if blender allows. Usually for normal python packages the top level should not include any python source files, instead they should be a src or cad_sketcher directory which contains all source files and a main.py entrypoint.

Yeah i'm aware of that and i agree it would be nice to move files away from the root folder. The reason they live in the root folder is to support the git installation workflow mentioned here where the addon is placed in the scripts folder defined in blenders preferences. I'va already tried to simply add a link on the root level to the init.py which blender looks for but that didn't seem to work unfortunately.

Along with this i would discourage the use of relative imports, as they can lead to issues with modules on sys.path having the same name and are discouraged by PEP8.

True, i don't really see a reason against it

This all hinges on how blender works with addons, a topic which i have no experience with.

[bonjorno7]

Another topic to consider is structuring the repository differently, if blender allows. Usually for normal python packages the top level should not include any python source files, instead they should be a src or cad_sketcher directory which contains all source files and a main.py entrypoint.

Along with this i would discourage the use of relative imports, as they can lead to issues with modules on sys.path having the same name and are discouraged by PEP8.

This all hinges on how blender works with addons, a topic which i have no experience with.

Blender requires the folder that goes into scripts/addons/ to have an __init__.py file with bl_info. Blender addons only work with relative imports; we can't follow pep8 in this regard. You can minimize files in the top folder by putting the rest of the addon in a subfolder and importing it from the top init. I highly recommend you give each package and module (un)register functions, instead of overseeing registration from a central place. I'm fine with using Black, though I'm definitely a 120 line length enjoyer; 80 is just so restrictive.

[laundmo]

Blender addons only work with relative imports; we can't follow pep8 in this regard.

im curious about this: how does blender load addons? how can blender affect the choice of importing used?

[bonjorno7]

Blender addons only work with relative imports; we can't follow pep8 in this regard.

im curious about this: how does blender load addons? how can blender affect the choice of importing used?

I guess the addon's package isn't the top package, but one that is imported by Blender from higher up. Therefore its submodules don't exist at the top level, but rather as addon_name.sub_module in sys.modules.

[Cheaterman]

Let's clarify status on this :-) - do you consider #229 to be finished? Is there a WIP PR to split out utils/constants or is this a potential point where I could get started?

I'm not entirely sure I'll be able to singlehandedly add all necessary type hints and docstrings but a best effort will be made :-) (I think my time is best spent on implementing new features in the first place, then I'll use remaining available time to work on docs & types)

[hlorus]

Let's clarify status on this :-) - do you consider #229 to be finished?

Yes that's already done.

Is there a WIP PR to split out utils/constants or is this a potential point where I could get started?

This is also already done, mostly in this commit: https://github.com/hlorus/CAD_Sketcher/commit/b73d41b1ce5749abb988533e4bb6f215e468353e#diff-8656a273c89d5812379185ade358273fa3d93e2cb8df832c66cec928eef9809a

I'm not entirely sure I'll be able to singlehandedly add all necessary type hints and docstrings but a best effort will be made :-) (I think my time is best spent on implementing new features in the first place, then I'll use remaining available time to work on docs & types)

IMO this is just an ongoing topic that can be improved when doing cleanups or refactors.

What's mainly remaining here is to split ui.py and workspacetools.py into individual files and maybe move all keymap definitions to one place e.g. keymaps.py.

[hlorus]

Also if you encounter stuff that could use a cleanup/refactor, here would be a good point to report that