Skip to content

Conversation

@danking
Copy link
Contributor

@danking danking commented Oct 30, 2025

I find myself wanting this in Spiral. Is it reasonable?

I find myself wanting this in Spiral. Is it reasonable?

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking danking requested a review from a10y October 30, 2025 21:13
@danking danking marked this pull request as ready for review October 30, 2025 21:14
Signed-off-by: Daniel King <dan@spiraldb.com>
@a10y
Copy link
Contributor

a10y commented Oct 30, 2025

why is the as_struct_fields version not good enough here?

@danking
Copy link
Contributor Author

danking commented Oct 30, 2025

We maybe shouldn't do this but there's cases where I get an owned DType and I want to convert to a Spiral Schema which is a StructFields without nullability (b/c its a SQL-style table: no top-level nulls).

@danking
Copy link
Contributor Author

danking commented Oct 30, 2025

I currently do as_struct_fields(...).cloned()

@danking danking added the changelog/feature A new feature label Oct 30, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 30, 2025

CodSpeed Performance Report

Merging #5128 will not alter performance

Comparing dk/into-struct-fields (3ae901a) with develop (e1ca80a)

Summary

✅ 1325 untouched
⏩ 99 skipped1

Footnotes

  1. 99 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

❌ Patch coverage is 0% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.76%. Comparing base (8945cd8) to head (3ae901a).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-dtype/src/dtype.rs 0.00% 33 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@a10y
Copy link
Contributor

a10y commented Oct 30, 2025

I think this is fine, maybe we should add owned into_ variants for the other conversion methods too

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking
Copy link
Contributor Author

danking commented Oct 31, 2025

I added an into_ method for each of the per-variant as_ methods.

Signed-off-by: Daniel King <dan@spiraldb.com>
@danking danking enabled auto-merge (squash) October 31, 2025 19:38
@danking
Copy link
Contributor Author

danking commented Oct 31, 2025

all checks passing now.

@danking danking merged commit 9c4ecb6 into develop Oct 31, 2025
39 checks passed
@danking danking deleted the dk/into-struct-fields branch October 31, 2025 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants