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

Refactored set fulltexts #62

Merged
merged 5 commits into from
Mar 18, 2018
Merged

Refactored set fulltexts #62

merged 5 commits into from
Mar 18, 2018

Conversation

ebreton
Copy link
Contributor

@ebreton ebreton commented Mar 17, 2018

Hello @dragonleman ,

Je suis repassé sur set_fulltext pour la rendre plus pythonesque, et ajouter des tests unitaires qui explicitent le comportement (tu verras, j'y ai mis des pdf, pdfa, et aussi ppt et asp...)

En revanche, j'ai vu que tu ne l'appelais plus nulle part dans le code ... ?

on avait avant dict_result['ELA_URL'] = set_fulltext(fulltexts), mais il a sauté au commit 8377ea4

normal ?

(il y a aussi un peu de corrections flake8 dans le commit)

@ebreton ebreton requested a review from dragonleman March 17, 2018 20:58
@dragonleman
Copy link
Contributor

dragonleman commented Mar 17, 2018

Je me demande comment l'appel à la fonction set_fulltext ait pu disparaître ainsi. La fonction doit être appelée :


def get_ELA_fields(field):
ela_fulltexts = []
ela_icon = ''
for ela in field:
ELA_type = ela.get('x', '').lower()
if ELA_type == 'icon':
ela_icon = ela.get('u', '')
elif ELA_type == 'public':
ela_fulltexts.append(ela.get('u', ''))
ela_fulltexts = list(filter(None, ela_fulltexts))
ela_fulltexts = set_fulltext(ela_fulltexts)
return {'icon': ela_icon, 'fulltexts': ela_fulltexts}


C'est la raison pour laquelle j'avais mal interprété tes ".0" à chaque appel à article.ELA_URL dans les templates: je pensais que tu les avais supprimés, pas ajoutés ...

Copy link
Contributor

@dragonleman dragonleman left a comment

Choose a reason for hiding this comment

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

Seulement l'appel à get_fulltext. Et, à vérifier, tous les appels dans les templates à ELA_URL, où il faudra enlever les ".0".

@ebreton
Copy link
Contributor Author

ebreton commented Mar 18, 2018

Aucune idée de comment ça a disparut... 😯

Mais en tout cas, la ligne n'était pas là dans le commit qui a ajouté la fonction (8377ea4#diff-75a2b8ce6d351c48326a1234d952edc4R112), et il n'y a eu ensuite qu'une seule modif sur ce code (flake8) qui ne l'a ni ajoutée ni enlevée 😬

peut-être juste une mauvaise manip. Mais ce n'est pas vraiment important maintenant qu'on a identifié le soucis ☺️

En tout cas c'est mieux sans les .0 que j'ai rajoutés partout... c'était pas très propre

Je vais pousser le code qui rajoute l'appel à set fulltext et faire marche arrière sur les .0

@ebreton ebreton closed this Mar 18, 2018
@ebreton ebreton reopened this Mar 18, 2018
@ebreton
Copy link
Contributor Author

ebreton commented Mar 18, 2018

voilà, @dragonleman je te laisse me dire si on est bon comme ça :)

@ebreton ebreton merged commit 82961a9 into master Mar 18, 2018
@ebreton ebreton deleted the refactored-set-fulltexts branch March 27, 2018 14:53
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.

2 participants