Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Projeto de IC #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

GabrielaKucuruza
Copy link

@GabrielaKucuruza GabrielaKucuruza commented Jun 19, 2022

  • Atualização e adição das PNADs 2003, 2004, 2005, 2007, 2009, 2011, 2012, 2013, 2014 e 2015
  • Adição da POF 2017-2018
  • Correção de erros

Gabriela Kucuruza, João Oliveira e Tayara Causanilhas

Copy link
Owner

@arademaker arademaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. os arquivos .RData e .Rhistory não deveriam estar incluídos no repositório.
  2. o arrive le.pesquisa.R foi totalmente modificado? No PR é importante descrever o que foi alterado e o motivo.
  3. não entendi a estrutura das novas pesquisas adicionadas. Até 2012 eu tinha um arquivo .rda por PNAD. Cada .rda é um dataset e um dataset pode conter um ou mais data.frames. Na documentação, temos arquivos .Rd para o dataset e para cada data.frame. Vocês mantiveram esta estrutura? O que vcs mudaram nos arquivos .Rd já existentes? O que fizeram para as novas pesquisas? Parece que nas novas pesquisas não temos apenas dois arquivos para ler, correto?
  4. Por que temos nomes como dic_dom_2003 e também dicdom1983?
  5. Por que termos rot_10_pof_201 _2018.Rd e também rot9pof2008.2009.Rd? Por que não usar sempre o mesmo padrão de nomes?

@@ -0,0 +1,3 @@
exportPattern("^[^\\.]")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O que esta linha significa?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Esta linha já estava no código antes da modificação. O que foi inserido foram os imports necessários para corrigir os erros presentes do NOTE, conforme explicitamos no issue no nosso repositório.

De toda forma, esta linha refere-se à definição das variáveis, de forma que o pacote exporta tudo a partir do namespace que não começa com um ponto.

@@ -0,0 +1,69 @@
le.pesquisa <- function(dicionario, pathname.in, codigos, rotulos = NULL, tbloco = 2000, nlines = NA) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seria bom detalhar no PR o que foi modificado neste arquivo. Descrever de forma clara o que precisou ser mudado. Estou achando estranho TODAS as linhas terem sido modificadas. Foi isso mesmo? Ou talvez isto só tenha sido causado por vcs editarem este arquivo no Windows?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O código não foi modificado, aconteceu provavelmente por conta do Windows mesmo.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nao foi isso. parece que foi mesmo erro de como @GabrielaKucuruza iniciou o repositório dela. Tem algo muito errado ... melhor falarmos urgente.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qual erro?

Eu estou no trabalho. Não sei a disponibilidade dos demais para uma reunião.

Maintainer: Erick Fonseca <[email protected]>
Description: This package contains dictionaries for reading microdata
Title: Dictionaries for reading microdata surveys from IBGE
Version: 1.6
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a versão deve mudar para 1.7, certo? Vcs fizeram mudanças, não pode continuar sendo 1.6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Versão alterada.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

não vejo ainda... cuidado! Estou achando que vcs partiram de versão antiga do repositório... melhor conferir antes de corrigir os problemas.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A versão alterada está no repositório do nosso projeto, referente a nossa matéria.

Version: 1.6
Date: 2014-08-13
Author: Alexandre Rademaker <[email protected]>
Maintainer: Alexandre Rademaker <[email protected]>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existe algum campo para contribuidor? Se sim, vcs poderiam colocar o nome de vocês.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Já foram alteradas a data e a versão.

@arademaker
Copy link
Owner

Notem que eu não posso fazer o merge porque o PR está em conflito com a versão dos arquivos no branch master. Vcs criaram o branch de trabalho a partir do master? Vejam conflitos acima... resolvam os conflitos e problemas que apontei para que eu possa aceitar o PR.

@arademaker arademaker changed the title Projeto de IC - Gabriela Kucuruza, João Oliveira e Tayara Causanilhas Projeto de IC Jun 22, 2022
@arademaker
Copy link
Owner

Os issues #5 e #3 foram resolvidos no projeto?

@arademaker
Copy link
Owner

Talvez vcs tenham feito confusão e feito clone do repositório do Erick, que está 12 abandonado... Ele começou o pacote quando trabalhava comigo... mas apenas começou e depois eu assumi a manutenção.

@@ -1,16 +1,16 @@
Package: dicionariosIBGE
Type: Package
Title: Dictionaries for reading survey microdata from IBGE
Version: 1.1
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a versão no master do arademaker/dicionariosIBGE é 1.6. Como aqui está aparecendo a versão 1.1?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essa foi a alteração feita hoje:

Screen Shot 2022-06-22 at 12 31 46

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reforço que não estou conseguindo entender como este PR foi feito. Não vou conseguir avaliar o projeto de vocês.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boa tarde @arademaker lamento a demora, estava sem acesso a computador até o presente momento. Entendi agora que houve uma confusão na realização do PR e peço desculpas, me orientei erroneamente pela data da versão do erick, que aparecia de 8 anos atrás. Para evitar os conflitos, queria confirmar com o senhor se posso realizar um novo PR que esteja alinhado com a última versão do pacote. Fico no aguardo e agradeço desde já pela compreensão.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obs: como @tayaracausanilhas fez atualizações no repositório do emap-ic, para evitar os problemas que tive, fiz um fork do seu repositório e então adicionei todos o nosso projeto, na minha conta pessoal. Eu realizaria o PR a partir daqui, https://github.com/GabrielaKucuruza/dicionariosIBGE,

image

@tayaracausanilhas
Copy link

  • Por que temos nomes como dic_dom_2003 e também dicdom1983?
  • Por que termos rot_10_pof_201 _2018.Rd e também rot9pof2008.2009.Rd? Por que não usar sempre o mesmo padrão de nomes?

Os nomes já foram alterados para o padrão antigo.

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.

3 participants