KOLANICH-ML / UniOpt.py

Unified interface for hyperparams / black box optimizers !!! Migrated to Codeberg 🏔️ !!!
https://codeberg.org/KOLANICH-ML/UniOpt.py
The Unlicense
3 stars 1 forks source link

About lack of PEP-8 #1

Closed impredicative closed 1 year ago

impredicative commented 5 years ago

Just to be sure, do you actually expect users to professionally ever use this code if you so vehemently ignore PEP-8? I wouldn't count on them using any PEP-8 non-compliant API.

Can you think of a major popular Python package that is equally PEP-8 non-compliant? I can't.

KOLANICH commented 5 years ago

Hi. Thank you for the interest in my library.

About pep-8.

  1. pep 8 is not a law of the nature.
  2. pep 8 is not the best possible solution. For example it requires using 4 spaces instead of a tab. It means
    • 4 times more bytes spent on indentation
    • editing problems: when you hit a backspace, 1 space is removed, which breaks indentation.
  3. people must do as well, as possible.

The only excuse about using spaces in languages allowing tabs I know is that "someone's editor is set to have tabs 4 spaces wide, someones prefer them be 2 spaces wide, so the code would look very different".

We don't have this problem:

  1. we use .editorconfig. It is specified that a tab is 4 spaces wide in it. Everyone having the editor understanding it will have tabs 4 spaces wide.
  2. indentation of functions and operators arguments based on size of function name is forbidden. In that case one should move arguments on own lines and indent with a single tab per a logical block.

I also quite dislike mandatory spaces around operators, thay make the code lines look too long. I like it better if they looked more condensed, so I usually don't insert them.

I dislike the convention about 79 characters per line. Really, 79 char per line in 2021, when everyone has a wide display more than 20'', and even if (s)he doesn't, text editors has the feauture to wrap long lines in order to fit their display / window size?

So I don't consider pep8 as convenient and I don't think I must follow pep8 only because someone follows it.

One can just apply autopep8 to own fork and enjoy it, but it would likely create problems when merging, so it's not the thing someone is expected to do. In fact code style is not the main thing in any sensible lib for any sane developer and it doesn't worth the prohlems it creates. But it's me the main developer currently, so I should have the code in the state convenient for me. So the style won't change. If you dislike it, use autopep8.

So

Can you think of a major popular Python package that is equally PEP-8 non-compliant? I can't.

Surely I can, if the community is at least a bit sane (it is not a verified fact, for example JS community in general doesn't look like sane, but I hope python community is).

I wouldn't count on them using any PEP-8 non-compliant API.

It's your own choice and your own problems.

About the sutability for professional use.

The project is alpha quality currently. Though I use it in my own projects, like HDDModelDecoder and AutoXGBoost, I constantly rewrite the master branch because I find ways to improve it and I don't want to keep these commits because they consume space. The history consumes damn too much space to keep the commits noone will need (remember, I'm the only dev of this lib for now, so no need to preserve authorship information - I'm the only author).

IMHO in its current state of the lib it is OK to use it in projects needing some (it means ~1 test run with small number of iterations just to make sure that the things work for your case) brief human supervision in order to work, like the daily work done by a data scientist - the mainnuse case for this lib. So if something breaks, you can investigate the problem and fix it.

Definitely not suitable for the ones who cannot investigate and fix issues, by the lib target audience are programmers, they can do it and they do it every day ;).

For example I'm not happy with the current architecture and I gonna change it. I wanna add the possibility to:

As you see, pretty lot of work, which likely will result in some API changes.

As usual, any help is welcome, but please keep in mind some important facts:

  1. the main repo is on GitLab, because I like its CI (Travis is really broken currently, they use the unsupported linux distro) and dislike acquisition of GH by M$ It is GitHub again. 2 the main branch will be often rewritten. It doesn't mean that you can't send PRs. 3 I don't have much time for it for now. I have done it because in one of one my researches. In fact I have not planned it, just one piece of AutoXGBoost grew large enough so I have separated it into a separate library (I felt like hyperopt takes too long).

So, if you are not ready to use it right now and are not ready to read all the stuff going on here, you can use GH beta feature of subscribtion only to releases.

impredicative commented 5 years ago

I appreciate that you're an independent thinker. PEP-8 allows for a configuration file to selectively exclude specific rules. I never meant for it to be followed to the letter. The main concern is about naming conventions. The public API of your package must at a minimum follow PEP-8 naming conventions for it to be used. These conventions apply to package, module, class, method, and attribute names.

KOLANICH commented 5 years ago

The main concern is about naming conventions.

Thank you for the clarification.

What's wrong with camelCase? It requires 1 character less per a word than snake_case. So it is faster to type and results in smaller files. And it is used for example by PyQt and PySide, it is popular and everybody is OK with it. So why not?

impredicative commented 5 years ago

Module, class, and instance names in PyQt5 look PEP-8 compliant. If you want to use camel case for method names, fine, but there is still a good reason to keep module, class, and instance names compliant.

KOLANICH commented 5 years ago

Which names do you propose to change?

Classes names look pep8-compliant. Functions and variables names are not, but we have discussed that we keep them camelCase. Modules names are definitely non-compliant. But should they be? Is uniopt or uni_opt better than UniOpt? Should we rename HyperparamVector.py into hyperparam_vector? From one side it reduces ambiguity and name collisions, so we may really should. From other side I feel like some module names would be better if they were written in the class-like style.

MSpec is a factory function, so it is more like a class, that's why it is named as a class. (BTW, the name is not good, we need to change it).

HyperparamsSpecsConverters should really have the first letter lowecase, because it is used as a dict, not as a class, so it is more like a static variable.