search

jumipeca / Ahorro_Combustible

Saving money in Fuel vehicle
0 stars 0 forks source link

Feat/ac 2 #3

Closed jumipeca closed 5 years ago

jumipeca commented 5 years ago

Changes to "ahorro_combustible/utils.py" implemented

jumipeca commented 5 years ago

I have had to give up to find a way to count the brands in a "naive" or "obvius" way and I have taken advantage from the "collections object" to do it. Test it and tell me your opinion.

jumipeca commented 5 years ago

I have run py.test and I have found a problem with the call "import collections", I had put the "import" in the wrong code. Now it's fixed. There is only one problem left.

imagen

ixemad commented 5 years ago

The error commented in your previous comment was because of the Counter.most_common() method returns equal counts in arbitrary order. I modified the docstring to pass that test but it could fail randomly.

ixemad commented 5 years ago

This problem may be solved using just a dictionary. The advantange of using the Counter class is that getting the value of an item that does not exist in the counter return 0. Using a dictionary, you have to handle that situation as follows:

cnt_brand_dict = {}
for brand in lst_tup_pet_sts:
    cnt_brand_dict[brand] = cnt_brand_dict.get(brand, 0) + 1

I prefer your way.

ixemad commented 5 years ago

Running the perfomance test with timeit returns theses figures

Execution time for size 100 was 0.00025635699967097025
Execution time for size 1000 was 0.0021211539997239015
Execution time for size 10000 was 0.022209803999430733
Execution time for size 100000 was 0.19749572399996396
Execution time for size 1000000 was 1.9144447819999186 Execution time for size 10000000 was 19.335614487001294

The execution time is linear instead of exponential.

jumipeca commented 5 years ago

Git rebase-squash applied to clean the branch

jumipeca commented 5 years ago

"git rebase-squash" applied. Last 3 commits squash to one.

ixemad commented 5 years ago

Buuueno ya casi está. Mi única pega es que el mensaje del commit "Function test implemented https://github.com/jumipeca/Ahorro_Combustible/pull/3/commits/c004b38d890907156655bab504f6da1e12e477cd" no es indicativo de la funcionalidad que implementa. Yo utilizaría como base en título de issue #2. Para cambiar un mensaje de un commit. Utiliza commit --amend. Como el cambio está publicado en github tendrás que subirlo con git push -f.

¡Abrazos!

El mié., 9 oct. 2019 a las 14:00, Juan M. (notifications@github.com) escribió:

"git rebase-squash" applied. Last 3 commits squash to one.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/jumipeca/Ahorro_Combustible/pull/3?email_source=notifications&email_token=AEN4UASNYJPWB273IANVRU3QNXBWVA5CNFSM4IX4JW6KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAXUREY#issuecomment-539969683, or mute the thread https://github.com/notifications/unsubscribe-auth/AEN4UAV3TBCLCG3TO7NNBETQNXBWVANCNFSM4IX4JW6A .

jumipeca commented 5 years ago

git rebase -i HEAD~1 // reword issue #2 Done!!!