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

Baixar datasets relacionados #26

Merged
merged 8 commits into from
Aug 2, 2019

Conversation

nymarya
Copy link
Member

@nymarya nymarya commented Aug 1, 2019

Adicionada a função `download_related_datasets, que recebe uma palavra-chave de interesse.

Copy link
Member

@itepifanio itepifanio left a comment

Choose a reason for hiding this comment

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

A mensagem de nenhum conjunto de dados dá um espaço muito grande:

Captura de tela de 2019-08-01 14-05-08

Se puder deixar numa linha só mesmo

@nymarya nymarya requested a review from itepifanio August 1, 2019 17:19
Copy link
Member

@itepifanio itepifanio left a comment

Choose a reason for hiding this comment

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

Tudo certo, nada errado 👍

Copy link
Member

@diegodiogenes diegodiogenes left a comment

Choose a reason for hiding this comment

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

Show de bola 👍

Copy link
Member

@alvarofpp alvarofpp left a comment

Choose a reason for hiding this comment

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

Trocar def _levenshtein(self, str1, str2): por def _levenshtein(self, str1: list, str2: list) -> float:, para manter as boas práticas.

Não acho que foi necessário a quebra de linha na função download_datasets, visto que estava dentro da quantidade de caracteres por linha definido pelo PEP. Mas isso é frescura minha mesmo.

@alvarofpp
Copy link
Member

Estava pensando em duas coisas aqui que acho que seria interessante debater.

Levenshtein

É um conjunto de duas funções, uma que realiza o cálculo (_levenshtein) e outra que aplica o filtro (_search_related_datasets). Creio que seria interessante torna isso em um mixin e implementá-lo na classe Dataset, ao invés de serem funções da dita classe. Dessa forma, podemos aplicar isso para filtrar tanto os datasets, como os grupos (fará mais sentido após a leitura do tópico a seguir).

Fluxo

Creio que poderíamos alterar essa issue para se tornar uma função que retorna uma lista com os datasets relacionados, porém não que realiza o download. Pensei nisso devido que não tenho o controle ou garantia que os datasets que estão sendo baixados serão justamente os que eu queria. E se ao invés de todos os datasets, eu querer excluir um? Não consigo fazer isso com a função. O dataset que não quero será baixado da mesma forma. Se aplicarmos uma função que retorna a lista de datasets, porém não baixa eles direto, eu tenho o poder de decidir o que será ou não baixado.

discentes_datasets = ufrn_data.related_datasets('discente')
ufrn_data.download_datasets( discentes_datasets[:-1] )

Dessa forma, poderíamos expandir isso para ter uma função que procura os grupos relacionados a sua entrada (por isso faz sentido transformar em um mixin, como disse no tópico anterior).

@alvarofpp
Copy link
Member

@nymarya @itepifanio @diegodiogenes Comentem sobre o que abordei anteriormente.

@nymarya
Copy link
Member Author

nymarya commented Aug 1, 2019

Trocar def _levenshtein(self, str1, str2): por def _levenshtein(self, str1: list, str2: list) -> float:, para manter as boas práticas.

Não acho que foi necessário a quebra de linha na função download_datasets, visto que estava dentro da quantidade de caracteres por linha definido pelo PEP. Mas isso é frescura minha mesmo.

Pelo que achei e pelo plugin que eu uso, o máximo de caracteres é 79 (https://www.python.org/dev/peps/pep-0008/#maximum-line-length)

@alvarofpp
Copy link
Member

Trocar def _levenshtein(self, str1, str2): por def _levenshtein(self, str1: list, str2: list) -> float:, para manter as boas práticas.
Não acho que foi necessário a quebra de linha na função download_datasets, visto que estava dentro da quantidade de caracteres por linha definido pelo PEP. Mas isso é frescura minha mesmo.

Pelo que achei e pelo plugin que eu uso, o máximo de caracteres é 79 (https://www.python.org/dev/peps/pep-0008/#maximum-line-length)

Ah, pois show. É porque eu uso o da JetBrains, aí ele deixa uma linha marcando, mas deve ser outro limite.

@itepifanio
Copy link
Member

Estava pensando em duas coisas aqui que acho que seria interessante debater.

Levenshtein

É um conjunto de duas funções, uma que realiza o cálculo (_levenshtein) e outra que aplica o filtro (_search_related_datasets). Creio que seria interessante torna isso em um mixin e implementá-lo na classe Dataset, ao invés de serem funções da dita classe. Dessa forma, podemos aplicar isso para filtrar tanto os datasets, como os grupos (fará mais sentido após a leitura do tópico a seguir).

Gostei da ideia do mixin e do fluxo levado em consideração, porém acredito que isso pode ser feito durante o desenvolvimento dessa outra issue.

Fluxo

Creio que poderíamos alterar essa issue para se tornar uma função que retorna uma lista com os datasets relacionados, porém não que realiza o download. Pensei nisso devido que não tenho o controle ou garantia que os datasets que estão sendo baixados serão justamente os que eu queria. E se ao invés de todos os datasets, eu querer excluir um? Não consigo fazer isso com a função. O dataset que não quero será baixado da mesma forma. Se aplicarmos uma função que retorna a lista de datasets, porém não baixa eles direto, eu tenho o poder de decidir o que será ou não baixado.

discentes_datasets = ufrn_data.related_datasets('discente')
ufrn_data.download_datasets( discentes_datasets[:-1] )

Dessa forma, poderíamos expandir isso para ter uma função que procura os grupos relacionados a sua entrada (por isso faz sentido transformar em um mixin, como disse no tópico anterior).

Quanto a baixar sem ter realmente um arquivo, para ser integrado mais facilmente com notebooks eu acho uma ótima ideia, quando pensei no pacote era mais para baixar os arquivos sem ter que ir clicando na interface, mas isso deixa o programador com a tarefa de recuperar os arquivos, dessa forma que tu propõe é bem interessante para protótipação e análises rápidas. Quando abrir a issue discutimos se utilizaremos flags para indicar essa nova funcionalidade ou uma função nova.

@diegodiogenes
Copy link
Member

diegodiogenes commented Aug 2, 2019

Estava pensando em duas coisas aqui que acho que seria interessante debater.

Levenshtein

É um conjunto de duas funções, uma que realiza o cálculo (_levenshtein) e outra que aplica o filtro (_search_related_datasets). Creio que seria interessante torna isso em um mixin e implementá-lo na classe Dataset, ao invés de serem funções da dita classe. Dessa forma, podemos aplicar isso para filtrar tanto os datasets, como os grupos (fará mais sentido após a leitura do tópico a seguir).

Gostei da ideia de expandir para um mixin.

Fluxo

Creio que poderíamos alterar essa issue para se tornar uma função que retorna uma lista com os datasets relacionados, porém não que realiza o download. Pensei nisso devido que não tenho o controle ou garantia que os datasets que estão sendo baixados serão justamente os que eu queria. E se ao invés de todos os datasets, eu querer excluir um? Não consigo fazer isso com a função. O dataset que não quero será baixado da mesma forma. Se aplicarmos uma função que retorna a lista de datasets, porém não baixa eles direto, eu tenho o poder de decidir o que será ou não baixado.

discentes_datasets = ufrn_data.related_datasets('discente')
ufrn_data.download_datasets( discentes_datasets[:-1] )

Dessa forma, poderíamos expandir isso para ter uma função que procura os grupos relacionados a sua entrada (por isso faz sentido transformar em um mixin, como disse no tópico anterior).

Acho que além dos datasets e grupos relacionados, isso poderia ser expandido para a lista dos próprios grupos em si, por exemplo, um cara quer o grupo de ensino, mas não quer os datasets de empréstimos da biblioteca. Então antes de realizar o download, poderíamos listar os datasets de cada grupo.

@alvarofpp @nymarya @itepifanio o que acham? Sugiro quebrarmos em novas issue também, claro.

@alvarofpp alvarofpp merged commit c742f60 into master Aug 2, 2019
@alvarofpp alvarofpp deleted the issue_14-baixar_datasets_relacionados branch August 2, 2019 17:52
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.

4 participants