Skip to content
New issue

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

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

Already on GitHub? # to your account

Add date parameter occurrences #84

Merged
merged 18 commits into from
Dec 12, 2023
Merged

Conversation

FelipeSBarros
Copy link
Owner

@cuducos aproveitei o embalo e comecei a trabalhar no parâmetro de data para o end point occurrences, conforme mencionado em #83 .

@FelipeSBarros FelipeSBarros requested a review from cuducos December 6, 2023 02:05
Copy link
Collaborator

@cuducos cuducos left a comment

Choose a reason for hiding this comment

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

Uma coisa, só para não esquecer. Precisamos validar e documentar o formato de entrada da data — isso ajuda a quem for usar a entender melhor possíveis erros. A não ser que a resposta da API com uma data formatada de forma errada seja bem boa : )

@FelipeSBarros
Copy link
Owner Author

Uma coisa, só para não esquecer. Precisamos validar e documentar o formato de entrada da data — isso ajuda a quem for usar a entender melhor possíveis erros. A não ser que a resposta da API com uma data formatada de forma errada seja bem boa : )

Perfeito. A validação eu fiz ontem. o request tem que ser feito com 2023-07-13 e initial_date < final_date. Mas, de fato, podemos aceitar outros formatos e trabalhar "internamente".
Sobre documentar, vc diz que colocar no readme todas as possibilidades possíveis, confirma? Ou haveria alguma outra forma de documentar?

@cuducos
Copy link
Collaborator

cuducos commented Dec 6, 2023

Sobre documentar, vc diz que colocar no readme todas as possibilidades possíveis, confirma?

Não necessariamente “com todas as possibilidades possíveis” — mas de uma forma que mencione claramente o formato da data esperado.

@cuducos
Copy link
Collaborator

cuducos commented Dec 6, 2023

A validação eu fiz ontem. o request tem que ser feito com 2023-07-13 e initial_date < final_date.

Só para avisar: talvez faltou o git push hehehe…

@FelipeSBarros
Copy link
Owner Author

A validação eu fiz ontem. o request tem que ser feito com 2023-07-13 e initial_date < final_date.

Só para avisar: talvez faltou o git push hehehe…

Não... é que eu fiz teste de chamada à API manualmente ó para ver o comportamento dela. Não por "pytest".

No último commit adicionei a lógica que vc havia proposto de limpar o dados que vem de data (usei expressão regular), para depois converter a datetime e valido se tenho tanto initial_date e final_date e se initial_date > final_date, retornando um DateError se for o caso.

Acho que deve ter uma forma mias "pythonista", usando menos "ifs". Imagino que vc sbaerá me induzir a u código mais interessante...

Copy link
Collaborator

@cuducos cuducos left a comment

Choose a reason for hiding this comment

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

Acho que existem uns espaços para melhorias pequenas, o que acha?

Comment on lines +84 to +89
| Name | Required | Description | Type | Default value | Example |
|---|---|---|---|---|---|
| `state_id` | ❌ | ID of the state | string | `None` | `'b112ffbe-17b3-4ad0-8f2a-2038745d1d14'` |
| `city_id` | ❌ | ID of the city | string | `None` | `'88959ad9-b2f5-4a33-a8ec-ceff5a572ca5'` |
| `city_name` | ❌ | Name of the city | string | `None` | `'Rio de Janeiro'` |
| `format` | ❌ | Format of the result | string | `'dict'` | `'dict'`, `'df'` or `'geodf'` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Foi falha minha, mas se todas são string, precisamos da coluna Type?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Talvez em citites não seja necessário. Mas em occurrences, sim. Pois o parâmetro id_citites pode ser uma string ou lista de strings...
Faz muita diferença se deio assim?
Pois , ainda que tenhamos a coluna "exemplos", não é óbivio que o id_cities seja uma string.

Copy link
Collaborator

@cuducos cuducos left a comment

Choose a reason for hiding this comment

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

🚀

@FelipeSBarros FelipeSBarros merged commit 8755490 into master Dec 12, 2023
@FelipeSBarros FelipeSBarros deleted the add_date_parameter_occurrences branch December 12, 2023 17:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants