Fixing datasciencebr/serenata-toolbox#5 and datasciencebr/serenata-toolbox#7 issues

okfn-brasil / serenata-toolbox

📦 pip module containing code shared across Serenata de Amor's projects | ** Este repositório não recebe atualizações frequentes **

MIT License

154 stars 69 forks source link

Fixing datasciencebr/serenata-toolbox#5 and datasciencebr/serenata-toolbox#7 issues #6

Closed jonDel closed 7 years ago

jonDel commented 7 years ago

Pull content:

Fixing init file;
Removing unused imports and local variables.
- Fixing script xml2csv.py location and reference in setup.py References:
  datasciencebr/serenata-toolbox#5 datasciencebr/serenata-toolbox#7

cuducos commented 7 years ago

Isn't it more coherent to adapt xml2csv.py to a module/class and use it within other scripts (instead of running it as a script)?

jonDel commented 7 years ago

I did not realize that the script was called from inside a python module. @cuducos , indeed that call seems a bit strange. I thought that script was meant to be used as a tool to be invoked from command line. If that was not the original intention, and it was meant only to be used inside that module, I think is better refactor the script into a class/function inside that module.

jonDel commented 7 years ago

By the way, I can do that. No problems. @Irio , what do you think?

Irio commented 7 years ago

@jonDel That would be ideal. Go for it.

cuducos commented 7 years ago

That'd be awesome, @jonDel. Many thanks. Also you've removed the hello_world but the docs at README.md are still referring to it. Either we keep that dummy function as a way to test if the module was installed and loaded into the sys.path or we have to update the docs. Does that make sense?

jonDel commented 7 years ago

I'll update that either.

jonDel commented 7 years ago

@Irio, the requested changes were made.

jonDel commented 7 years ago

It is ok to be merged?