Skip to content
This repository has been archived by the owner on Mar 1, 2018. It is now read-only.

Simplify classifiers code #50

Open
lipemorais opened this issue May 16, 2017 · 4 comments · Fixed by #89
Open

Simplify classifiers code #50

lipemorais opened this issue May 16, 2017 · 4 comments · Fixed by #89

Comments

@lipemorais
Copy link
Contributor

I think that the classifiers code are very complex, and it's not because of data science context knowledge necessary to understand it. I'm afraid that few people will be able to maintain it in a near future.

There is some magic numbers, a considerably amount of method chains and big methods in some classifiers.

https://github.com/datasciencebr/rosie/tree/master/rosie/chamber_of_deputies/classifiers

@lipemorais
Copy link
Contributor Author

lipemorais commented Oct 19, 2017

I would like to address this issue but I need help with what that code are trying to say.

I believe that solve this issue will help with transparency making easier for people understand each classifier and will help people contribute with classifier as well.

My plan is address one classifier at time:

Who I believe that can help with this?
@cuducos @anaschwendler @jtemporal @Irio

I will keep this TODO list update as long as I open the PRs. :)

@anaschwendler
Copy link
Collaborator

Hi @lipemorais thanks for that. I really believe that this might help a lot!

I'll help you refactoring the classifiers, and my suggestions is one PR for each one, and that we keep testing in small bytes, always checking if the number os suspicions for each classifier remain the same in the way to that refactor.

Please any doubt, say here and we keep a conversation that everyone can participate 🎉

@lipemorais
Copy link
Contributor Author

I will start with election_expenses_classifier.py.

As soon as I have some doubt I will put it here. :)

@cuducos
Copy link
Collaborator

cuducos commented Nov 4, 2017

Closed automatically, reopened because we still have refactors to do ; )

@cuducos cuducos reopened this Nov 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants