How to get rid of pep8 error E266 in FEM?

[vocx]

Would it be possible to show how you would make it, on an example def in this file https://github.com/FreeCAD/FreeCAD/blob ... cxtools.py ?

Wow. This uses the Doxygen style very well. It's good if you plan to use Doxygen exclusively, but I'd prefer the Python style.

Code: Select all

    ## Sets CalculiX ccx binary path and validates if the binary can be executed
    #  @param self The python object self
    #  @ccx_binary path to ccx binary, default is guessed:
    #  "bin/ccx" windows, "ccx" for other systems
    #  @ccx_binary_sig expected output form ccx when run empty.
    #  Default value is "CalculiX.exe -i jobname"
    def setup_ccx(self, ccx_binary=None, ccx_binary_sig="CalculiX"):
        error_title = "No CalculiX binary ccx"
        error_message = ""
        from platform import system
        ccx_std_location = FreeCAD.ParamGet(
            "User parameter:BaseApp/Preferences/Mod/Fem/Ccx"
        ).GetBool("UseStandardCcxLocation", True)

Becomes this

Code: Select all

    def setup_ccx(self, ccx_binary=None, ccx_binary_sig="CalculiX"):
        """Set Calculix binary path and validate its execution.

	Parameters
	----------
	ccx_binary : str, optional
	    It defaults to `None`. The path to the `ccx` binary. If it is `None`,
	    the path is guessed.
	ccx_binary_sig : str, optional
	    Defaults to 'CalculiX'. Expected output from `ccx` when run empty.

	Raises
	------
	Exception
        """
        error_title = "No CalculiX binary ccx"
        error_message = ""
        from platform import system
        ccx_std_location = FreeCAD.ParamGet(
            "User parameter:BaseApp/Preferences/Mod/Fem/Ccx"
        ).GetBool("UseStandardCcxLocation", True)

You are supposed to add things like Exceptions and return values if they exist. I think this is an excellent summary Example NumPy Style Python Docstrings. And also numpy docstring guide.

By the way, in those type of functions, I find it odd that the argument is a string, but the default is None. The default value should also be a string, with the default path.

Code: Select all

    def setup_ccx(self, ccx_binary="bin/ccx", ccx_binary_sig="CalculiX"):

[bernd]

Would it be possible to show how you would make it, on an example def in this file https://github.com/FreeCAD/FreeCAD/blob ... cxtools.py ?

Wow. This uses the Doxygen style very well. It's good if you plan to use Doxygen exclusively, but I'd prefer the Python style.

This was not added by me. I prefere the Python on too. I would like to switch to the Python one. That is why I am gone ask.

Thanks for providing the example.

Ahh I have another one. See files in this directory. Most of them have a structure like this file: https://github.com/FreeCAD/FreeCAD/blob ... tSource.py How do I use the Python style here ? Line 27 and 38 give a pep8 error.

[vocx]

How do I use the Python style here ? Line 27 and 38 give a pep8 error.

You can't. That instruction ##\addtogroup FEM is added on purpose in order to include this particular function in the documentation of FEM. I don't know if it's really necessary, to be honest, as Doxygen should automatically find and document each file.

My suggestion would be to remove one symbol, # \addtogroup FEM. This will stop the PEP8 errors. But then you need to test if the Doxygen documentation with make DevDoc is correctly generated. If it's correctly generated, then there is no need to use \addtogroup. However, if the class no longer appears in the documentation, that means the instruction was added specifically to be picked up by Doxygen; in this case there is not much to do except leave it there.

[bernd]

https://github.com/FreeCAD/FreeCAD/comm ... diff=split

but how about these ones ... https://github.com/FreeCAD/FreeCAD/blob ... #L229-L284

[vocx]

but how about these ones ... https://github.com/FreeCAD/FreeCAD/blob ... #L229-L284

I think these are class variables, right? In that case, they should be documented in Numpy style at the beginning of the class, in the __init__ method, or in the class docstring. They could still be documented in the code, but then it's not necessary to use the Doxygen style, with double ## Hashes.

[bernd]

but how about these ones ... https://github.com/FreeCAD/FreeCAD/blob ... #L229-L284

I think these are class variables, right? In that case, they should be documented in Numpy style ... , or in the class docstring. They could still be documented in the code, but then it's not necessary to use the Doxygen style, with double ## Hashes.

makes sense to me too and is much better to read if someone would like to use the class for the first time.

[bernd]

git commit a1e6fce

[bernd]

What is the difference between:

https://github.com/FreeCAD/FreeCAD/blob ... Control.py

Code: Select all

## \addtogroup FEM
#  @{

module code

##  @}

and
https://github.com/FreeCAD/FreeCAD/blob ... tential.py

Code: Select all

## @package ViewProviderFemConstraintElctrostaticPotential
#  \ingroup FEM
#  \brief FreeCAD FEM view provider for constraint electrostatic potential object

module code

no more comments on module end

[vocx]

What is the difference between:
...

I... don't know.

I think the only way of really knowing is to test it. Basically, create a group, use \addtogroup in some files, and \ingroup in others, with @{ and @}, and without, and see what type of documentation is produced with doxygen Doxyfile.

I would like to do these tests, but it's a lot of work, and testing "live", with the FreeCAD source code, is also hard because it's a very extensive code base, so any changes trigger a lot of changes in the produced Doxygen documentation. Therefore, this should be tested on a small scale, but I haven't had much time to do this since I rewrote Doxygen.

I have the impression that probably @{ and @} aren't needed, but they were added in the past to many source files to force Doxygen picking the code up. As with many things in FreeCAD, there is no clear explanation, and people just copied things from previous source files.

[bernd]

most of them where added with git commit af97583

Even back than both where used ...