Moving biometrics-related functionality to bob.bio.base
Merge request reports
Activity
mentioned in merge request bob.bio.base!138 (merged)
It is not a good idea to change the coding style of a package and also add/remove features in one merge request. It makes it impossible to review because there are so many changes (like here where you have changed 142 files).
The best thing to do is to do only style changes without changing anything else. Open a merge request for that one and we can merge it if it is green and the style is agreed upon.
Then, you open another merge request with minimal changes that is only changing functionality of the package.
Just an FYI, we try to follow numpy doc style for writing documentation in the code: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_numpy.html#example-numpy
-
You can also use
yapfto format the files automatically: https://github.com/google/yapf -
I can see that you are not just removing functionality with this MR. You are also adding support for a new score file format. This is not good. In order to review the merge request better and faster, you need to address one issue per merge request. Here, you are addressing #39 (moved) and #25 (closed) at the same time.
-
@theophile.gentilhomme I think I have read this page at some point: https://nvie.com/posts/a-successful-git-branching-model/
You can also leave the code style as it is. I don't think that I need to mention that I disagree with the 4 byte indentation style. When we started to implement all these packages, we have agreed on using two spaces for indentation. I don't know why you want to change this now. I know that this is the standard in PEP whatsit, but we all agreed to ignore this since we all hated it.
-
@mguenther, I have asked around here and most people (@andre.anjos @pkorshunov myself @onikisins ...) prefer 4 spaces.
I think we need to run something like
yapfon our code automatically to stop these formatting holy wars.YAPF takes a different approach. It's based off of 'clang-format', developed by Daniel Jasper. In essence, the algorithm takes the code and reformats it to the best formatting that conforms to the style guide, even if the original code didn't violate the style guide. The idea is also similar to the 'gofmt' tool for the Go programming language: end all holy wars about formatting - if the whole codebase of a project is simply piped through YAPF whenever modifications are made, the style remains consistent throughout the project and there's no point arguing about style in every code review.
-
Hi @theophile.gentilhomme this looks good to me (LGTM). Thank you. Please merge it if you are done.
Note to others: This is not being merged into master since we want to release Bob 3.1 before merging this.
Thanks @amohammadi.