Cria parte do utilitário para incluir DOI nos registros h e f de uma data bases#717
Conversation
Cria o módulo cisis e testes para gerar comandos para atualizar uma dada base com dados de DOI
robertatakenaka
changed the title
robertatakenaka
changed the title
robertatakenaka
changed the title
patymori
left a comment
patymori
left a comment
There was a problem hiding this comment.
Fiz somente uma observação. Tentei rodar o teste com o comando descrito no PR mas não funcionou. Tive que informar o módulo tests.test_cisis especificamente para rodar.
| Retorna a "proc" que atualiza | ||
| os campos v91 (data) e v92 (hora) da atualização | ||
| """ | ||
| now = datetime.now().isoformat() |
There was a problem hiding this comment.
Ao invés de retirar os traços e dois pontos do resultado de isoformat, não seria melhor usar strftime e definir o formato que você precisa?
|
@robertatakenaka valiadndo esse PR. |
gustavofonseca
left a comment
gustavofonseca
left a comment
There was a problem hiding this comment.
A fim de facilitar a compreensão daqueles que não entendem de procs do cisis, sugiro que os valores utilizados como exemplos nos testes de unidade sejam modificados para que fiquem mais parecidos com os dados reais (fazendo com que DOIs se pareçam com 10.1590/1809-4392201902421 ao invés de foo).
| ) | ||
|
|
||
|
|
||
| def proc_update_datetime(file_path): |
There was a problem hiding this comment.
Se essa função passar a receber a data como argumento, ela se tornará uma função pura e será mais fácil testá-la. Não será mais necessário o uso de patch, por exemplo.
| db = "base" | ||
| file_path = "xml/a.xml" | ||
| h_mfn = "4" | ||
| doi = "DOI" |
There was a problem hiding this comment.
No uso real o valor da variável doi seria algo como 10.1590/1809-4392201902421 ?
| h_mfn = "4" | ||
| doi = "DOI" | ||
| doi_items = [ | ||
| ("pt", "doi_PT"), |
There was a problem hiding this comment.
No uso real os valores do índice 1 de cada tupla seria algo como 10.1590/1809-4392201902421-en?
|
Olhando a descrição do tíquete e a implementação não encontrei no código o trecho que realizar o "desligamento" da criação de DOIs por idioma, se existir poderia me indicar? Veja no descrição da atividade, #713:
|
|
Estou tentando realizar os testes porém estou recebendo o seguinte erro:
Verifiquei que no arquivo de setup.py indica que o python deve ser em python >= 3.6. Para resolver esse problema de rodar os testes verifiquei que é necessário criar o init.py dentro da pasta de tests, veja:
Aí os testes rodaram, veja:
|
| @@ -0,0 +1,48 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Recomendo uma limpeza de comentário e variáveis que não estão sendo utilizadas no setup.py
jamilatta
left a comment
jamilatta
left a comment
There was a problem hiding this comment.
Poderia, por gentileza, verificar os ajustes solicitados.
Por enquanto, isso é só parte do utilitário. Neste ponto só são algumas funções soltas. Não há nenhuma lógica, portanto não há neste momento o que ligar ou desligar. |
Remove a restricao de versao do python
|
@scieloorg/scielo-brazil-developers podem revisar novamente? |
patymori
left a comment
patymori
left a comment
There was a problem hiding this comment.
O código está OK, só precisa corrigir o teste que falhou:
Para um passo futuro, também seria bom incluir a execução dos testes unitários no CI.
There was a problem hiding this comment.
@robertatakenaka todas as recomentações de ajuste que fiz ainda não foram realizadas sobre o arquivo init_.py para rodar os testes e a limpeza no setup.py.
4 participants
O que esse PR faz?
Cria o módulo cisis e testes para gerar comandos para atualizar uma dada base com dados de DOI (veja a proposta completa (não aprovada) em #713)
Onde a revisão poderia começar?
por commits
Como este poderia ser testado manualmente?
Dentro
proc/scielo_doi, execute:python setup.py testAlgum cenário de contexto que queira dar?
n/a
Screenshots
n/a
Quais são tickets relevantes?
#713
Referências
n/a