Skip to content

IpeaData: Package review#3

Open
joaoavf wants to merge 5 commits intoipea:masterfrom
joaoavf:review
Open

IpeaData: Package review#3
joaoavf wants to merge 5 commits intoipea:masterfrom
joaoavf:review

Conversation

@joaoavf
Copy link

@joaoavf joaoavf commented Aug 23, 2018

Review of the whole ipeadata Python package.

My review needs to be carefully reviewed as I made some major suggestions without understanding the package so well.

Please let me know what do you think of this contribution so I can evaluate if I will invest more time in this or otherwise.

I will be available to solve issues related to this PR and I am interested in making future contributions as I intend to use this package.

@gutorc92
Copy link
Collaborator

João, muito obrigado pela contribuição. Eu tenho alguns comentários a fazer.

Os comentários ficaram muito descritivos e bem melhores do que os anteriores. A importação no init também facilitara bastante o uso do pacote. Eu agradeço bastante a contribuição e faço alguns comentários para que o merge request possa ser aceito:

  1. Devido a manter uma compatibilidade com o pacote do cran que se chama ipeaData o mesmo foi mantido para o pacote em python.

  2. Para valorizar o nome da instituição procuramos sempre manter o nome Ipea em evidência portanto não deveriamos mudar o nome do arquivo de ipeadata para api.

@joaoavf
Copy link
Author

joaoavf commented Aug 28, 2018

Gustavo, que bom que você gostou do PR! Fiz as alterações que você pediu.

Tem isso nas docstrings:

    1. FNTID: <FILL EXPLANATION>
    2. FNTSIGLA: <FILL EXPLANATION>
    3. MACRO: <FILL EXPLANATION>
    4. REGIONAL: <FILL EXPLANATION>
    5. SOCIAL: <FILL EXPLANATION>

Você tem isso bem documentado em algum lugar para eu atualizar esse PR com as informações corretas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants