Skip to content

Conversation

@charles-turner-1
Copy link
Collaborator

@charles-turner-1 charles-turner-1 commented Sep 1, 2025

Change Summary

  • In Add parquet driver #728, I added support for reading .parquet files, but completely forgot to add support for serialising catalogs back to them. This PR fixes that.
  • Cleans up an issue with available compression associated with that PR.
  • Tests we can deserialise and reserialise (from {.csv,.parquet} => {.csv, .parquet}).
  • Adds write_kwargs, a la read_kwargs in Add parquet driver #728, plus soft deprecation of the to_csv_kwargs keywods argument when serialising.

N.B: Deserialisation & reserialisation tests round trip twice, it looks like the defaults for how empty catalog field options (in catalog.json) have changed since that catalog was created.

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI
  • Documentation reflects the changes where applicable
    • Will need to update ecgtools first to allow for quickstart guide update.
    • Relevant API reference bits updated.

I will no longer make pull requests when jetlagged.

Add note on default's that might have changed?
@mgrover1
Copy link
Collaborator

@charles-turner-1 - let me know when you would like a review here

@charles-turner-1
Copy link
Collaborator Author

Sorry Max, completely forgot to reply to you here!

We've been using a fork the past month or so to allow us to go further/faster with development on this - I think there are a couple of minor tweaks there that need merging back into here. I'll ping you for a review once I'm satisfied that everything is working as intended!

(N.B I'm mostly treating our fork as something of a staging area - intention is to sync developments back in here.)

@mgrover1
Copy link
Collaborator

No worries @charles-turner-1 👍 sounds like a plan!!

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