Skip to content

Parse hours, SIREN and SIRET as text#42

Merged
AntoineAugusti merged 8 commits intomasterfrom
parse-time
Sep 4, 2019
Merged

Parse hours, SIREN and SIRET as text#42
AntoineAugusti merged 8 commits intomasterfrom
parse-time

Conversation

@AntoineAugusti
Copy link
Member

@AntoineAugusti AntoineAugusti commented Sep 3, 2019

Related to #41.

Detect values that should be treated as strings and force a string casting. This PR introduces a custom type checker for agate to recognise hours, SIREN and SIRET.

@AntoineAugusti
Copy link
Member Author

agatesql needs a patch because of this exception:

Error parsing CSV: Unsupported column type: <csvapi.type_tester.Time object at 0x7fb516c307d0>

@abulte
Copy link
Contributor

abulte commented Sep 3, 2019

Cool! This means we can write a SIREN/SIRET detector and stop parsing them as integer 💪

@AntoineAugusti
Copy link
Member Author

Yup, but it requires patching agatesql as well. For now it doesn't seem possible to append a custom type to the type => SQL type mapping defined.

What's the problem with SIREN or SIRET parsed as integers?

@abulte
Copy link
Contributor

abulte commented Sep 3, 2019

Leading 0

@abulte
Copy link
Contributor

abulte commented Sep 4, 2019

I guess we can fork agate-sql and make a PR upstream to account for custom types.

@AntoineAugusti AntoineAugusti changed the title Parse time as text instead of datetime Parse hours, SIREN and SIRET as text Sep 4, 2019
@AntoineAugusti AntoineAugusti marked this pull request as ready for review September 4, 2019 07:51
@AntoineAugusti
Copy link
Member Author

@abulte Should I merge this as it is?

Should I add things to the CHANGELOG / change other files?

@abulte
Copy link
Contributor

abulte commented Sep 4, 2019

@AntoineAugusti Add a line in CHANGELOG and then LGTM

LGTM

@AntoineAugusti AntoineAugusti merged commit d840eb4 into master Sep 4, 2019
@AntoineAugusti
Copy link
Member Author

:shipit:

@AntoineAugusti AntoineAugusti deleted the parse-time branch September 4, 2019 09:49
This was referenced Mar 3, 2020
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