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

[Revisão retroativa] Imprensa Oficial #1125

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

claromes
Copy link
Contributor

@claromes claromes commented Apr 4, 2024

Layout do site publicador de diários oficiais

Marque apenas um dos itens a seguir:

  • O layout não se parece com nenhum caso da lista de layouts padrão
  • É um layout padrão e esta PR adiciona a spider base do padrão ao projeto junto com alguns municípios que fazem parte do padrão.
  • É um layout padrão e todos os municípios adicionados usam a classe de spider base adequada para o padrão.

Código da(s) spider(s)

  • O(s) raspador(es) adicionado(s) tem os atributos de classe exigidos.
  • O(s) raspador(es) adicionado(s) cria(m) objetos do tipo Gazette coletando todos os metadados necessários.
  • O atributo de classe start_date foi preenchido com a data da edição de diário oficial mais antiga disponível no site.
  • Explicitar o atributo de classe end_date não se fez necessário.
  • Não utilizo custom_settings em meu raspador.

Testes

Verificações

  • Eu experimentei abrir alguns arquivos de diários oficiais coletados pelo meu raspador e verifiquei eles conforme a documentação não encontrando problemas.
  • Eu verifiquei os arquivos .csv gerados pela minha coleta conforme a documentação não encontrando problemas.
  • Eu verifiquei os arquivos de .log gerados pela minha coleta conforme a documentação não encontrando problemas.

Descrição

Issue: #951

Lista:

  • Base
  • Amélia Rodrigues BA
  • Conceição do Almeida BA (Mudou para o sistema BR Transparência)
  • Gentio do Ouro BA
  • Gongogi BA (Mudou para o sistema SAIIO)
  • Governador Mangabeira BA
  • Itaquara BA (Mudou para o sistema BR Transparência)
  • Jaguarari BA
  • Muniz Ferreira BA
  • Paratinga BA
  • Pé de Serra BA
  • São Felipe BA
  • São Félix BA
  • São Francisco do Conde BA
  • São Miguel das Matas BA
  • Sapeaçu BA
  • Saúde BA (Mudou para o sistema BR Transparência)
  • Serrinha BA
  • Vera Cruz BA
  • Wenceslau Guimarães BA
  • Xique-Xique BA

Comentários

  • As cidades não selecionadas acima, não foram testadas, porém, já estão com os metadados atualizados para a Base revisada
  • Nova issue ([Novo spider base]: BR Transparência #1146) criada listando as cidades que usam o sistema BR Transparência, incluindo Conceição do Almeida BA, Itaquara BA e Saúde BA

@trevineju
Copy link
Member

trevineju commented Apr 19, 2024

@claromes, obrigada por estar fazendo essa tarefa!

As coisas importantes a ser feitas são:

  1. Testar a classe base, que depende de alguns municípios
  2. Para os casos que ainda tem diários, mas não são mais atuais, adicionar o end_date e modificar o nome.
  3. Para os casos que o site não mais existe ou não tem mais diários, excluir o arquivo do repositório. (A gente não tem por quê manter o código de raspagem de um site que não tem mais diários)

Pelo que vejo desta PR, ela já cumpre...

  • o item 1: com os checks de Base, Amélia Rodrigues, Gentio do Ouro e Governador Mangabeira.
  • o item 2: com os checks de Itaquara e Saúde.

... e falta ajustar:

  • o item 3: remover o arquivo ba_conceicao_do_almeida.py

Também vou pedir que retire os comentários de situação. Por exemplo, pegando Itaquara:
The files between the years 2019 and 2022 are available using the Imprensa Oficial system. This city now uses the BR Transparência system, which covers other cities. The Base is not developed.

O fato do raspador estar com start_date, end_date e herdar a base ImprensaOficialSpider já evidencia a informação da primeira linha, então é redundante. E, quanto a segunda, caberia mais abrir uma issue para o novo padrão que achou e listar todos os casos que já viu fazer parte do padrão.

Com isso, pode tirar todas as demais cidades dessa PR. E justando esse pouquinho, já estaria pronta pra revisão.

💯

@trevineju
Copy link
Member

trevineju commented Apr 19, 2024

Outra coisa... @claromes, você tinha perguntado sobre fazer duas PRs. Tô supondo que uma sendo essa PR nesse ponto que parou e a próxima cobriria as demais cidades, certo?

Você até poderia fazer isso, sim. Mas, também, pode só partir pra outra se quiser. Tô dizendo isso pq o complexo você já fez aqui que era validar a classe base, o resto seria só ajustar as classes-filhas e executar testes. Costumo sugerir esse tipo de tarefa super fácil pra pessoas novas na comunidade ou iniciantes querendo contribuir pela primeira vez.

Pro seu caso, por ser mais experiente, é mais interessante fazer outra issue, como uma que adiciona outra nova classe base. Isso também é mais interessante pra gente, visto que cada base nos possibilita adicionar dezenas ou centenas de novas cidades.

Sua escolha, claro ;)

@claromes claromes marked this pull request as ready for review May 21, 2024 00:44
@claromes
Copy link
Contributor Author

@trevineju Obrigada pelos comentários. Fiz as alterações e só testei três cidades que mudaram. Deixei alguns comentários na descrição do PR.
Pronto para revisão! \o/

@trevineju trevineju linked an issue May 29, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

[Revisão retroativa] Imprensa Oficial
2 participants