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

Ajoute un test de présence pour le fichier epcicom2020.xlsx #198

Conversation

Shamzic
Copy link
Contributor

@Shamzic Shamzic commented Dec 15, 2023

@Shamzic Shamzic self-assigned this Dec 15, 2023
@Shamzic Shamzic force-pushed the 1529-tester-le-chargement-du-fichier-epcicom2020xlsdans-la-réform-epci branch from 8496505 to b084f13 Compare December 15, 2023 13:47
Copy link
Contributor

@Allan-CodeWorks Allan-CodeWorks left a comment

Choose a reason for hiding this comment

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

En tant que tel, je crois que le test n'apporte pas de valeur car si le fichier en question n 'est pas présent, le test de la réforme EPCI ne passera pas :/

Copy link
Contributor

Choose a reason for hiding this comment

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

Je crois que la problématique évoquée dans le ticket est de s'assurer que le chemin utilisé pour charger la réforme sera valable quand le module est utilisé.
Il ne s'agit pas de tester la présence du fichier mais la validité du chemin.

À la base, j'ai voulu supprimer des avertissements de dépréciations dans la réforme EPCI sauf que j'ai remplacé la fonction dépréciée resources.path('openfisca_france_local', 'epcicom2020.xlsx') par une URI en dur pd.read_excel('openfisca_france_local/epcicom2020.xlsx')

Le problème c'est que le rôle de la méthode path() (remplacée actuellement par files('openfisca_france_local').joinpath('epcicom2020.xlsx')) est de lié un package à un chemin.

Autrement dit, quand on appelle openfisca dans le dépôt externe, le fichier epcicom2020.xlsx n'est pas présent dans l'arborescence du projet en question. Ce qu'on fait ici c'est de dire "va chercher le fichier epcicom2020.xlsx dans le dossier du package openfisca_france_local"

Le test à écrire est donc de vérifier que l'URI utilisée lorsque openfisca_france_local est utilisée en tant de que dépendance ne dépend pas de l'arborescence du projet en question...

@Allan-CodeWorks Allan-CodeWorks force-pushed the 1529-tester-le-chargement-du-fichier-epcicom2020xlsdans-la-réform-epci branch from b084f13 to 4202324 Compare January 2, 2024 13:54
@Allan-CodeWorks Allan-CodeWorks self-assigned this Jan 2, 2024
@Allan-CodeWorks Allan-CodeWorks requested a review from a team January 2, 2024 14:19
@Allan-CodeWorks
Copy link
Contributor

Il semblerait que j'ai trouvé un moyen de tester :
par défaut le dossier d'exécution des tests est la racine du dépôt.
En changeant le chemin d'exécution avec chdir vers un dossier temporaire généré par pytest dans /tmp.

Pour vérifier que ça marche bien j'ai pris l'ancienne version de la reforme qui ne marchait pas et on voit bien que le test échoue avec cette version mais pas avec la nouvelle.

@Shamzic
Copy link
Contributor Author

Shamzic commented Jan 2, 2024

Je suis à l'origine de la PR du coup je peux pas approuve : ok pour moi concernant la modif avec le test du chemin via un dossier temporaire en python @Allan-CodeWorks

@Allan-CodeWorks Allan-CodeWorks force-pushed the 1529-tester-le-chargement-du-fichier-epcicom2020xlsdans-la-réform-epci branch from 4202324 to d2f0cc8 Compare January 8, 2024 13:02
@Allan-CodeWorks
Copy link
Contributor

En attente de régler les problèmes de déploiement sur Pypi.

@Allan-CodeWorks Allan-CodeWorks force-pushed the 1529-tester-le-chargement-du-fichier-epcicom2020xlsdans-la-réform-epci branch from d2f0cc8 to dd51f3b Compare January 18, 2024 10:40
@Allan-CodeWorks Allan-CodeWorks merged commit c7587da into master Jan 18, 2024
11 checks passed
@Allan-CodeWorks Allan-CodeWorks deleted the 1529-tester-le-chargement-du-fichier-epcicom2020xlsdans-la-réform-epci branch January 18, 2024 10:55
@guillett guillett added this to the BC actuel milestone Mar 22, 2024
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