feat: Add Schema and Field withMetadata methods for metadata replacement#334
feat: Add Schema and Field withMetadata methods for metadata replacement#334kentkwu wants to merge 2 commits intoapache:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds ergonomic withMetadata() helpers to Arrow JS Field and Schema so callers can replace or clear metadata using a Map, plain object, or null, aligning the JS API more closely with other Arrow language bindings.
Changes:
- Add
Schema.withMetadata()to replace/clear schema metadata. - Add
Field.withMetadata()to replace/clear field metadata. - Add unit tests covering object/Map/null metadata replacement semantics.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/schema.ts |
Introduces withMetadata() on Schema and Field, plus a helper to normalize inputs into a metadata Map. |
test/unit/schema-tests.ts |
Adds Jest unit tests for the new withMetadata() helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * @param metadata Replacement metadata entries. Pass `null` to clear. | ||
| */ | ||
| public withMetadata(metadata: Map<string, string> | Record<string, string> | null): Schema<T> { |
There was a problem hiding this comment.
I'm not familiar with JavaScript/TypeScript but it seems that we don't have withXXX style API.
It seems that the below assign() merges something to the current schema. Is schema.assign(metadata) a natural API?
There was a problem hiding this comment.
Thanks for the feedback @kou. You're correct that withXXX isn't currently really present in this library.
The naming was chosen to align with other Arrow implementations — PyArrow uses .with_metadata(), Rust Arrow uses .with_metadata(). In absence of another pre-existing convention here I’m proposing just adopting the same from the other bindings.
The existing assign() method does merge metadata from another Schema, but it doesn't cover the replace-metadata case without constructing a throwaway Schema:
schema.assign(new Schema(schema.fields, newMetadata))And even that merges rather than replaces. The only clean way to replace metadata today is to construct a Schema directly, which requires using the full constructor.
I'd also like to add more small convenience methods over time to improve the usability of the library, and this is a first step in that direction.
There was a problem hiding this comment.
This should give you a schema or field with the metadata replaced:
let schemaWithoutMetadta = schemaWithMetadata.clone({ metadata: new Map })
let fieldWithoutMetadta = fieldWithMetadata.clone({ metadata: new Map })And if you want to mutate instead of clone, you can do schema.metadata.clear().
What's Changed
Summary:
Field.withMetadata()andSchema.withMetadata()so callers can replace or clear metadata maps without manually juggling Map<string,string> instances. Both helpers accept either a Map, a plain object, or null (to clear), and always return a new instance, matching the C++/PyArrow/Rust behavior.test/unit/schema-tests.tswith coverage for the new helpers. I couldn't find a better place to put these but let me know if I missed something.Closes #333