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

CSV seperators & dev activity #52

Closed
svenkerud opened this issue Aug 12, 2022 · 5 comments
Closed

CSV seperators & dev activity #52

svenkerud opened this issue Aug 12, 2022 · 5 comments

Comments

@svenkerud
Copy link

hi @pnadolny13,

You seem to be the most active maintainer on this project so I took the liberty to ping you.
I am wondering if there is currently any dev activity on this project? most of the commits apears to be dependabot generated.

If there is dev activity I was wondering if there is anything going on with regards to adding support for other csv sepperators, spessifically ;.

Thanks in advance for your help.

@pnadolny13
Copy link
Collaborator

Hey @svenkerud - yep I've been the primary maintainer. Happy to review PRs or help get new features added!

I think this is a reasonable and generically useful feature to add. In

reader = csv.reader(f)
we load the csv with csv.reader() which takes a bunch of optional arguments https://docs.python.org/3/library/csv.html#csv-fmt-params that we're not using right now. We could probably expose those to the tap config, maybe under a single csv_format object?

Is this something you'd consider contributing a PR for?

@svenkerud
Copy link
Author

Briliant thanks for the repy.
I am happy to look into this issue as this is deffinetly something we need.
I will ping you once i have a PR ready.

@pnadolny13
Copy link
Collaborator

@svenkerud a PR just merged to add configurable encoding #58. I think your new setting could basically replicate that same pattern. I take back what I said about using a single csv_format key. The files config dict at the file level is probably a fine place for it to go.

@Bartman0
Copy link
Contributor

Bartman0 commented Oct 8, 2022

I made #70 to implement the intended configuration. I needed it too, and I could not find a follow-up on this discussion

@pnadolny13
Copy link
Collaborator

@Bartman0 @svenkerud the PR was merged so I'm closing this issue. I will cut a new release, after I merge the dependabot PRs.

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

No branches or pull requests

3 participants