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

Adiciona a chamada de UF no cliente da API #24

Merged
merged 6 commits into from
Sep 15, 2023

Conversation

cuducos
Copy link
Collaborator

@cuducos cuducos commented Sep 14, 2023

Esse PR adiciona o endpoint states ao cliente da API:hon

>>> from crossfire.client import Client
>>> client = Client()
>>> states = client.states()
>>> states
                                     id            name
0  b112ffbe-17b3-4ad0-8f2a-2038745d1d14  Rio de Janeiro
1  813ca36b-91e3-4a18-b408-60b27a1942ef      Pernambuco
2  d3a9b545-7056-4dc6-9b68-ce320c9edffc           Bahia
>>> 

E adiciona a opção para não utilizar o Pandas caso não seja desejado:

>>> from crossfire.client import Client
>>> client = Client()
>>> states = client.states(format='dict')
>>> states
[{'id': 'b112ffbe-17b3-4ad0-8f2a-2038745d1d14', 'name': 'Rio de Janeiro'}, {'id': '813ca36b-91e3-4a18-b408-60b27a1942ef', 'name': 'Pernambuco'}, {'id': 'd3a9b545-7056-4dc6-9b68-ce320c9edffc', 'name': 'Bahia'}]

No caminho:

  • Corrige autenticação (faltava o Bearer no cabeçalho)
  • Move a função auxiliar to_dataframe para o módulo do cliente (creio que o load_data deve sumir já já)
  • Arruma o to_dataframe (retorna um DataFrame com os dados de data na resposta, não com a resposta toda)

Comment on lines -40 to +33
logging.info("Extracting data from Fogo Cruzado's API...")
client = load_client()
resp = client.get(url="https://api.fogocruzado.org.br/api/v1/cities")
return to_dataframe(resp)
return extract_data_api("https://api.fogocruzado.org.br/api/v1/cities")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Essa mudança não era absolutamente necessária, mas o corpo de ambas as funções era o praticamente o mesmo, então reduzi a repetição utiliando a extract_data_api para executar a extract_cities_api.

Esse arquivo vai sumir já já quando criarmos os métodos cities e occurrences no cliente.

Copy link
Owner

Choose a reason for hiding this comment

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

ok

Comment on lines -9 to +12
Token,
CredentialsNotFoundError,
IncorrectCrdentialsError,
Token,
UnknownFormatError,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Apenas organizei em ordem alfabética pois estava me perdendo me pergntando já importamos isso

Comment on lines -5 to +10
from crossfire.fogocruzado_utils import extract_data_api, extract_cities_api

from crossfire.load_data import InvalidDateIntervalError, get_fogocruzado
from geopandas import GeoDataFrame
from pandas import DataFrame

from crossfire.fogocruzado_utils import extract_data_api, extract_cities_api
from crossfire.load_data import InvalidDateIntervalError, get_fogocruzado

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Organizei os imports de acordo com a PEP8 pois estava me perdendo procurando as coisas importadas.

@cuducos cuducos mentioned this pull request Sep 14, 2023
15 tasks
Copy link
Owner

@FelipeSBarros FelipeSBarros left a comment

Choose a reason for hiding this comment

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

@cuducos , fiquei meio perdido. Se der para fazermos um review juntos rapidinho, pq não sei se pude me expressaar em minhas dúvidas...
De qq forma, aprovo e vo tentar estuda-lo melhor à tarde.

@@ -44,12 +74,20 @@ def __init__(self, **kwargs):

self.cached_token = None

@cached_property
def has_pandas(self):
Copy link
Owner

Choose a reason for hiding this comment

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

Entendo que essa propriedade checa se o pandas está instalado. Confirma?
Se não está instalaldo, não saltaria um erro jpa no import?

Copy link
Collaborator Author

@cuducos cuducos Sep 15, 2023

Choose a reason for hiding this comment

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

Se não está instalaldo, não saltaria um erro jpa no import?

Esse é um ótimo ponto. Na verdade eu introduzi esse padrão adiantando o que sugeri sobre o Pandas poder eventualmente não ser uma dependência. Também documentei essa ideia no meu rascunho #27.

Se formos para frente com essa ideia, vamos também mudar o import — vc prefere que eu já mude agora para algo como:

try:
    from pandas import DataFrame
    has_pandas = True
except ModuleNotFoundError:
    has_pandas = False

Dessa forma podemos matar esse método. O que acha?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sugeri e3f505e

Copy link
Owner

Choose a reason for hiding this comment

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

@cuducos concordo com essa proposta de ter o pandas como uma dependencia extra...

if format and format not in FORMATS:
raise UnknownFormatError(format)

response = method(self, *args, **kwargs)
Copy link
Owner

Choose a reason for hiding this comment

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

De onde vem este method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Vou usar as palavras de um amigo meu, excelente pessoa e com nível técnico avançado:

Ela recebe um method como parâmetro e é "empacotado" por um wrapper que pode ter parâmetros posicionais ou nomados...

Ou seja, quando eu uso:

@parse_response
def get(url):
    print(url)

method é a própria função get— o empacotamento do decorador com @parse_response seguido de def get na outra linhaé basicamenteparse_response(get)`

Repare que não estou executando a função get ainda, apenas passando o nome dela (uma referência a ela). Aí ela é executada no corpo da função (decorador) parse_response — bem na linha em q vc comentou : )

@@ -64,13 +102,19 @@ def token(self):
)
return self.cached_token.value

@parse_response
Copy link
Owner

Choose a reason for hiding this comment

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

Estw decorador chama a função criada antes, né?
Ela recebe um methodcomo parâmetro e é "empacotado" por um wrapper que pode ter parâmetros posicionais ou nomados...
É isso?


if "headers" not in kwargs:
kwargs["headers"] = auth
else:
kwargs["headers"].update(auth)

return get(*args, **kwargs)

def states(self, format=None):
return self.get(f"{URL}/states", format=format)
Copy link
Owner

Choose a reason for hiding this comment

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

Esse get é o que possui parse_response como decorador, confirma? vc está passando a url como parẫmetro posicional e o format, como nomeado.
Logo, format será usado no wrapper, quando informado. Mas ainda não entendi o method.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Esse get é o que possui parse_response como decorador, confirma?

Perfeito.

Logo, format será usado no wrapper, quando informado.

Exato, como documentei aqui. Alguma sugestão de melhoria para deixar isso mais claro?

Mas ainda não entendi o method.

Entendeu sim hahaha… vc descreve o que é o método nesse teu comentário. Vamos falar mais sobre ele pois posso estar interpretando errado : )

Comment on lines -40 to +33
logging.info("Extracting data from Fogo Cruzado's API...")
client = load_client()
resp = client.get(url="https://api.fogocruzado.org.br/api/v1/cities")
return to_dataframe(resp)
return extract_data_api("https://api.fogocruzado.org.br/api/v1/cities")
Copy link
Owner

Choose a reason for hiding this comment

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

ok

@FelipeSBarros FelipeSBarros merged commit a97e94d into FelipeSBarros:master Sep 15, 2023
1 check passed
@cuducos cuducos deleted the states branch September 25, 2023 13:31
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.

None yet

2 participants