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

Add support for container types (array, object) when reading JSON files #75

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

Conversation

amotl
Copy link
Contributor

@amotl amotl commented Dec 13, 2023

Dear Eric,

thanks a stack for conceiving this excellent tap.

We had the need to propagate container types (array and object) from JSON files unmodified, i.e. not serializing them to strings. This patch implements just that. While being at it, it also adds proper handling for boolean types.

With kind regards,
Andreas.

NB: A few test cases are failing: test_handle_newlines_local_excel, test_renamed_https_object, test_smart_columns. It looks like those failures are unrelated to the changes.

@rubenvereecken
Copy link

rubenvereecken commented Mar 6, 2024

We would benefit from this too.

Edit: now using this PR successfully.

@amotl
Copy link
Contributor Author

amotl commented Mar 26, 2024

Hi again. It looks like the branches diverged, and display conflicts. When there are interests to integrate this patch, we will be happy to resolve them.

@rubenvereecken
Copy link

@ets could you take a look at merging?

@ets
Copy link
Owner

ets commented Apr 9, 2024

@ets could you take a look at merging?

LGTM - Happy to merge once the PR is conflict free.

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