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

Features/wordcloud #123

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

Features/wordcloud #123

wants to merge 12 commits into from

Conversation

vitoryeso
Copy link
Contributor

Componente WordCloud. Pode receber textos em .csv ou apenas string. Possui alguns parâmetros como stopwords, max_words, background_color e etc. Também foram adicionados testes para esse componente.

@gvechini
Copy link
Contributor

gvechini commented Nov 8, 2021

Para todos os experiments, os títulos e descrições das seções poderiam ser mais claros e direcioandos para essa tarefa das componentes específicas, wordcould e pdf_extractor, eles estão como no defult de uma tarefa em branco.

Para todos os deployment, o titulo está como "Nova Tarefa - Implantação" e o texto está como "Preencha aqui com detalhes sobre a tarefa". Sugiro deixar esse header igual ao dos respectivos experiments.
No deploy de pdf_extractor tbm é preciso limpar os outputs, também seria melhor remover a última célula #testing.

@gvechini
Copy link
Contributor

gvechini commented Nov 8, 2021

No experiment do wordclous suba o aquivo com os outputs limpos.
No tópico "Leitura do conjunto de dados" a saída "data" tende a ser um vetor/coluna de csv bem grande né? Seria interessante mostrar só uma parte ou nem mostrar essa variável.

No experiment de pdf_extractor suba também o arquivo com os outputs limpos. Note que há uma caixinha de código não ultiliazda no final.
No tópico "Extraindo texto" você importa a função PDFExtractor e não ultiliza ela nesta célula, recomendo retirar essa chamada.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 8, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
8.9% 8.9% Duplication

from typing import List, Optional


def init_cut(string:str, delimiter: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Seria interessante adicionar um docstring explicando o que este método faz.

return delimiter + splitted[-1]
else: return "Delimiter not found."

def final_cut(string:str ,delimiter: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Seria interessante adicionar um docstring explicando o que este método faz. Assim como os outros métodos desenvolvidos.

else:
return None

def read_memory(stream,
Copy link
Contributor

Choose a reason for hiding this comment

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

Adicionar um docstring explicitando o que cada variável representa seria interessante.

Ex:

'''
Lê e pré processa os dados de um stream...

Parameters:
=========

    stream (tipo): Descrição...
    ... 
'''

@@ -340,8 +348,33 @@ def paracrawl_test_data():
"names":["text_english","text_portuguese"]
},
}
return data
return dat
Copy link
Contributor

Choose a reason for hiding this comment

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

Isso gerou um erro em outros componentes. O correto anteriormente escrito era return data

@lucasns97 lucasns97 self-requested a review November 25, 2021 12:34
Copy link
Contributor

@lucasns97 lucasns97 left a comment

Choose a reason for hiding this comment

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

Mencionei um ponto que está causando erro em outras tasks; e esta PR está causando conflito com o datasets.py, precisam ser resolvidos antes do merge

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.

3 participants