QB3 / sparse-ho

Fast hyperparameter settings for non-smooth estimators:
http://qb3.github.io/sparse-ho
BSD 3-Clause "New" or "Revised" License
39 stars 15 forks source link

Structure of the files #49

Closed Klopfe closed 3 years ago

Klopfe commented 3 years ago

The file models.py is getting big and I think we could create directories (models, criterion and algo) and split within each directory the files criterion and models into individual files for each model and criterion.

What do you think about that? Do you have suggestions?

mathurinm commented 3 years ago

I was thinking the opposite, merging the algo files. I prefer not to have 12 files open in my editor, when we do global renaming of a variable etc My 2 cents

Klopfe commented 3 years ago

It's also a possibilility

QB3 commented 3 years ago

I do not know what is the canonical way of doing it, but I would also tend to vote for splitting files. I addition of being conceptually clearer, I find the models.py file is becoming too large: https://github.com/QB3/sparse-ho/blob/master/sparse_ho/models.py#L1778

agramfort commented 3 years ago

I would split in

models/ base.py # containing BaseModel class lasso.py # importing from base the BaseModel to inherit logreg.py ...

Klopfe commented 3 years ago

Just to be sure, the methods of Lasso, SVM, etc... have the same name but are specific to each of the models, in that case do you implement an empty function in the parent class BaseModel with the same name? Or would we only put an init function and maybe some methods that are shared by all models without being specific ?

agramfort commented 3 years ago

any method that must be implemented by all inherited classes should be listed as I did in the abstract base class. From now on you know better then me what these methods should be.

the contract is that if a subclass implement these methods it will work

Klopfe commented 3 years ago

ok thanks