Skip to content

Conversation

@ehennestad
Copy link
Collaborator

@ehennestad ehennestad commented Oct 29, 2025

Motivation

  • Add comment providing context why a compound dataset with references needs to be loaded into memory when written to a new file
  • Reuse io.parseCompound to reduce code duplication and ensure processing of compound is standardised (the current version would not properly copy over boolean values contained in a compound dataset)
  • Update unittest for export/copy of DataStub with compound dataset to include a boolean data value, verifying that compound with references + booleans are correctly copied.

How to test the behavior?

Show here how to reproduce the new behavior (can be a bug fix or a new feature)

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.58%. Comparing base (7cdde97) to head (9673c1e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #767      +/-   ##
==========================================
+ Coverage   95.56%   95.58%   +0.01%     
==========================================
  Files         182      182              
  Lines        6386     6366      -20     
==========================================
- Hits         6103     6085      -18     
+ Misses        283      281       -2     

☔ 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.

@ehennestad ehennestad requested a review from Copilot October 29, 2025 22:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the DataStub export functionality to simplify compound data type handling with references by delegating postprocessing to a centralized function. The key improvement is reducing code duplication and ensuring consistent data type handling across the codebase.

  • Extracts reference checking logic into a helper function isCompoundWithReference
  • Replaces manual reference/character array processing with a call to io.parseCompound for consistency
  • Updates tests and schema to include a boolean field in compound types for better test coverage

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
+types/+untyped/@DataStub/export.m Refactored to extract reference checking logic and delegate compound data postprocessing to io.parseCompound
+tests/test-schema/compoundSchema/cs.compoundtypes.yaml Added boolean field 'c' to compound type schema for test coverage
+tests/+unit/dataStubTest.m Updated tests to include boolean field in compound data and improved test structure with renamed variables

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ehennestad ehennestad marked this pull request as ready for review October 29, 2025 22:21
@ehennestad ehennestad requested a review from bendichter October 29, 2025 22:37
@ehennestad ehennestad merged commit 541db1d into main Oct 30, 2025
18 checks passed
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