Add command line option to limit dataset years

[anaschwendler]

This is just a template to help you make your point clear with this PR. :)

What is the purpose of this Pull Request? Following @irio #60 with the option to limit dataset years, this PR:

• Update the way to select an specific path to save the datasets
• Fetch the Dataset() method for year, only if years is no None
• Update the README.md to this new option

What was done to achieve this purpose? Understand what @irio has done to achieve that new option and fix what was missing to make this new option works

How to test if it really works? Run rosie with a few options like:

$ python rosie.py run chamber_of_deputies --path /another/folder --years 2017
$ python rosie.py run chamber_of_deputies --path /another/folder
$ python rosie.py run chamber_of_deputies --years 2017,2016
$ python rosie.py run chamber_of_deputies 

$ python rosie.py run federal_senate --path /another/folder --years 2017
$ python rosie.py run federal_senate --path /another/folder
$ python rosie.py run federal_senate --years 2017,2016
$ python rosie.py run federal_senate 

Who can help reviewing it? @cuducos

[anaschwendler]

There will be some tests to add to rosie, but I think that it is a great start!

[cuducos]

There will be some tests to add to rosie, but I think that it is a great start!

Sure it is! Many thanks for that ; )

Two doubts:

1. Should we wait for these tests before reviewing the PR?
2. As the CLI gets more complicates I think we should adopt either argparse (⭐️ ), click (⭐️ ⭐️⭐️⭐️) or docopt ( ⭐️⭐️⭐️⭐️⭐️). What do you think of implementing that in this PR?

[anaschwendler]

Should we wait for these tests before reviewing the PR?

This feature is important, so I must consider accept as it is now, and open a new PR to that

As the CLI gets more complicates I think we should adopt either argparse (⭐️ ), click (⭐️ ⭐️⭐️⭐️) or docopt ( ⭐️⭐️⭐️⭐️⭐️). What do you think of implementing that in this PR?

I would like to do it now, but I think it is too much for one PR, but I can add too, or we can open an issue and I can do it following, what do you think @cuducos ?

[cuducos]

This feature is important, so I must consider accept as it is now, and open a new PR to that

If the feature is importante I'd say the opposite: let's not accept non-tested code ; ) Also, if the feature is important I think using argparse, click or docopt is very much welcomed: they are highly tested packages to avoid us reinventing the wheel and accidentally getting something wrong in in the way.

TLDR I honestly prefer tests and a high level CLI development tool before reviewing this PR bizarrely for the same reason you prefer to postpone tests and high level CI dev tool hahaha…

[anaschwendler]

Oh! Nice point, working on it!! \o\

[anaschwendler]

So considering what we need to do:

• [x] Test chamber_of_deputies with and without years
• [x] Include docopt to our CLI

[anaschwendler]

Now that I'm full recovered, I will get back to it!

[anaschwendler]

Just included docopt to our CLI, and had some doubts about documentation.

Docopt works fine, but when I run python rosie.py only, it gets a help > terminal, is it normal?

Independent of what I do, it don't change this option, what can be done about it?

[anaschwendler]

Just add the tests with and without years for chamber_of_deputies. The result is:

(serenata_rosie) ➜  rosie git:(anaschwendler-limit-years) ✗ python rosie.py test
..............................................................
----------------------------------------------------------------------
Ran 62 tests in 152.857s

OK

I'm completely sure that this it not the best way to do that, but I think that the logic is ok. I'll wait for @cuducos review, to see if he can help me with my tests.

[anaschwendler]

Hey @cuducos I think I got something here :)

I'm sure that there is a few things to fix, but its getting better, can you review for me?

[anaschwendler]

Hi @cuducos took a closer look on what you have said, and made some adjustments. I'm still not happy with the test script. But now the main script I think that may look nice. Can you review again for me?

[cuducos]

I'm still not happy with the test script.

What bugs you in this script?

[anaschwendler]

@cuducos It takes time by downloading the datasets from the source, I can't think of anything, but maybe there is another way to test that, without downloading the whole datasets

[cuducos]

there is another way to test that, without downloading the whole datasets

You could mock HTTP requests, or you could mock only the classes (as internally the way self.years works might be already tested).