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.

Sign up for GitHub

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

Jump to bottom

GEIQ : meilleure gestion du type de la colonne SIRET du xls importé #5297

Merged
merged 2 commits into from
Jan 6, 2025
Merged

GEIQ : meilleure gestion du type de la colonne SIRET du xls importé #5297

merged 2 commits into from
Jan 6, 2025

Conversation

EwenKorr
Copy link
Contributor

@EwenKorr EwenKorr commented Dec 20, 2024 •
edited
Loading

🤔 Pourquoi ?

Parfois les SIRET sont des float dans le tableur importé. Une fois casté en chaîne de caractères, le SIRET devient 'XXXXXXXXXXXXXX.0'.

cf https://inclusion.sentry.io/issues/15011873/

C'est la solution la plus courte que j'ai trouvée, mais je suis preneur d'amélioration le cas échéant !

Label bug ou pas ? 🤔

@EwenKorr EwenKorr added no-changelog Ne doit pas figurer dans le journal des changements. bug labels Dec 20, 2024
@EwenKorr EwenKorr self-assigned this Dec 20, 2024
xavfernandez
xavfernandez reviewed Dec 20, 2024
View reviewed changes
Copy link
Contributor

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

Je sais qu'il n'y a pas de tests sur cette commande, mais ça vaudrait peut-être le coup de commencer à en mettre, en commençant par tester juste la fonction get_geiq_df .
Tu as la fonction generate_excel_sheet pour créer des fichiers excel et la fixture tmp_path_factory pour stocker des fichiers temporaires 👼

itou/companies/management/commands/import_geiq.py Outdated Show resolved Hide resolved
@EwenKorr
Copy link
Contributor Author

Je commence à ajouter des tests donc. Et grâce à ça j'ai pu un peu mieux isoler le problème.

Avec le fichier actuel (2024-12-17 - Export BDD FFGEIQ.xls), voilà ce qui se passe :
si tous les SIRETS sont complets, pas de problème (en supprimant manuellement 2 lignes sans SIRET, ça passe)

Quand au moins 1 SIRET manque, la colonne prend le format XXXXXXXXXXXXXX.0 (je n'ai pas encore isolé à quelle étape).

@EwenKorr EwenKorr force-pushed the ewen/import_geiq branch 2 times, most recently from 128c70f to 5415f1a Compare January 6, 2025 11:13
@EwenKorr
import_geiq: add tests
9cc8548
@EwenKorr
import_geiq: force SIRET type to integer
0b84479 
When replacing NaN elements with None, if the siret column is not
explicitely defined as integer, it is converted to float.
These values later converted to string, need to be integers, otherwise
they are suffixed with `.0`.
@EwenKorr EwenKorr enabled auto-merge January 6, 2025 14:32
@EwenKorr EwenKorr added this pull request to the merge queue Jan 6, 2025
xavfernandez
xavfernandez reviewed Jan 6, 2025
View reviewed changes
Copy link
Contributor

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

Merci pour le fix et l'ajout de tests 🙏

@EwenKorr
Copy link
Contributor Author

EwenKorr commented Jan 6, 2025

Merci d'avoir pointé les éventuelles conséquences d'une valeur/date par défaut dans la définition d'une fonction !

Merged via the queue into master with commit d9e6c33 Jan 6, 2025
9 checks passed
@EwenKorr EwenKorr deleted the ewen/import_geiq branch January 6, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xavfernandez xavfernandez xavfernandez approved these changes

Assignees

@EwenKorr EwenKorr

Labels
bug no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@EwenKorr @xavfernandez