Coding standards: Function name aliases: how to use?

[Roy_043]

In the Draft code most functions have an alias:
joinWires = join_wires

Is this for compatibility? Where is this documented? And what should you use when you import Draft in your code? In the docstrings the camelCase name is used which suggests that that is the name to choose. In the scale.py module itself functions are called by their alias, strange, but it does confirm this idea.

In gui_scale.py there are a number of functions that do not have snake_case names. For some there may be technical reasons (GetResources and Activated) but in other cases it just seems inconsistent (scaleGhost and numericInput).

Aside:
The join_wires function (join.py) is a bit strange as it makes a recursive call to itself using its alias.

[vocx]

In the Draft code most functions have an alias:
joinWires = join_wires

Is this for compatibility?

Yes. Carlopav and I reworked the Draft Workbench earlier this year, and we renamed many functions to be PEP8 compliant. But to prevent breaking workbenches that depend on Draft (Arch and BIM mostly) and older scripts, providing the old name is necessary.

The PEP8 guidelines indicate that functions should be snake_case, so this should be the "official" name; the camelCase should be for compatibility purposes only. Obviously not every single function of the workbench has been renamed. This is a work in progress and takes a long time to complete; the documentation strings need to be updated as well, of course.

By the way, Carlo used a simple alias like what you wrote above. But lately I've been using a more clever way, creating the old function from the new function. So, we can actually inject a deprecation message, and the corresponding docstring.

Code: Select all

def getSVG(obj, args):
    """Return SVG string of the object. DEPRECATED. Use 'get_svg'."""
    utils.use_instead("get_svg")
    return get_svg(obj, args)

...
For some there may be technical reasons (GetResources and Activated) but in other cases it just seems inconsistent (scaleGhost and numericInput).

Those capitalized methods inside objects (draftobjects), viewproviders (draftviewproviders) and commands (draftguitools) are re-implementations of the C++ base class methods, so they must be named exactly like that (GetResources, Activated, Deactivated, etc.). But other methods should be renamed to comply with PEP8, as long as it doesn't affect compatibility. As soon as we detect incompatibilities, we should think about clever ways of handling that.

Unfortunately, there is no rule to know which methods come from C++, you have to check the C++ sources as well.

The join_wires function (join.py) is a bit strange as it makes a recursive call to itself using its alias.

This could be easily changed. As long as the alias is identical to the base function, it should work the same.

A lot of this is discussed in the big reorganization thread, [Discussion] Splitting Draft tools into their own modules.

But there is more to do. I'm going through various modules, fixing things, updating the docstrings, the algorithms where it makes sense, etc., but it takes a lot of time if you want to do it right. It's not just cosmetic issues, it's also about fixing small bugs, and other things that I notice in the code (for example, handle new objects that didn't exist before, handle App Links, etc.).

[Zolko]

In the Draft code most functions have an alias:

Code: Select all

joinWires = join_wires

Where is this documented?

I have the same question, please.

and we renamed many functions to be PEP8 compliant.
...
The PEP8 guidelines indicate that functions should be snake_case, so this should be the "official" name

PEP8 is for Python programs, but FreeCAD is not a Python program, some of it is C++, and a lot of calls go to Qt which is also C++ : therefore, a lot of functions and calls will never follow PEP8 guidelines. This means that FreeCAD has the choice to be either fully C++ like snakeOil, or to have a mix of C++ snakeOil and PEP8 snake_oil functions. But FreeCAD will never be all PEP8 compliant.

This is a work in progress and takes a long time to complete
...
Unfortunately, there is no rule to know which methods come from C++, you have to check the C++ sources as well.

to summarize: this is a huge work, purely cosmetic, brings confusion and no advantage. What is the purpose of it ?

[bernd]

I just can talk about FEM.

In FEM we did do this years ago. All C++ methods are camelCase als Python methods are snake_case (rigth fro the beginning so no compatibility probelems) and most code is pep8 compliant and and and. See https://github.com/FreeCAD/FreeCAD/blob ... entions.md

Why?

IMHO it is more than cosmethic. It is about consistency. If there is no unify code standard it is very very hard to read the code, to debug the code, etc. Much time in maintaining is fixing bug, fixing bugs in code other people made. If at leas this code is according a standard it is much simpler to read and to understand. Eventually this saves a lot of time.

Since we do not like to make our own standard we just used pep8.

m2c

[bernd]

When I started with FreeCAD we even had a lot of code files which had space and tab indent mixed, 2 space and 4 space indent. Windows and Unix line endings mixed in one file. Such stuff ist just a night mare to work with. Happily in FEM this is no longer the case.

[Zolko]

it is much simpler to read and to understand.

Since we do not like to make our own standard we just used pep8.

But any call to a FreeCAD function will be in the form of snakeOil:

Code: Select all

App.activeDocument().getObject('disk001')

All calls to Qt code will also have the same standard:

Code: Select all

RotXButton.setToolTip()
columnsLayout.addLayout()
attLCSlist.setMinimumHeight()

And there is nothing you can do about it. Therefore, by following PEP8 and its snake_oil() standard, you'll add different visuals to your code and reduce readability.

On the other hand, using snakeOil() for functions and some_variable for variables, it's very easy to see the difference between variables and functions in your code. In this way, all calls will be in the form:

Code: Select all

myGreatFunction(some_variable)

[vocx]

to summarize: this is a huge work, purely cosmetic, brings confusion and no advantage. What is the purpose of it ?

Python code of FreeCAD is still Python code, it can follow standards. You aren't thinking long term. We can refactor FreeCAD to have Python compliant functions, it just needs work, vanuan already provided some proposals on how to enforce this PEP8 checking. It's not extremely hard, it's just tedious work that somebody should do. Qt itself is moving towards more compatibility with Python, they said it themselves at the beginning of the year; it's possible that PySide6 for Qt6 will include a more Python compliant interface.

[bernd]

I must admit in FEM this is mixed ...

To be honest years ago code formating was a night mare, a coding standard was needed, pep8 seamed reasonable and bump here we are ....

FEM_Tutorial_Python

[Zolko]

to summarize: this is a huge work, purely cosmetic, brings confusion and no advantage. What is the purpose of it ?

You aren't thinking long term.

in other circumstances I would have stopped reading here, but I'll make an exception:

We can refactor FreeCAD to have Python compliant functions, it just needs work

you still didn't justify what the purpose of all this work is. All I can see is cosmetic waste of resources. And I certainly hope that no-one will allow you to waste everybody-else's time in refactoring FreeCAD code to have Python compliant functions.

[Kunda1]

Just want to interject here withmy moderator hat on and ask for a little de-escalation in the tone of this important exchange of ideas and long term vision.