Skip to content

Create append helper function and set $schema as first attribute - #366

Merged
peterdesmet merged 27 commits into
v2from
dollar-schema-first
Aug 29, 2026
Merged

peterdesmet merged 27 commits into
v2from
dollar-schema-first

Conversation

@peterdesmet

@peterdesmet peterdesmet commented Aug 26, 2026 •

Copy link
Copy Markdown
Member

Fix #358

Originally written by @PietrH in #362 Edited text from original PR:

I ended up implementing a generic S3 method for append() which allowed me to set a S3 method for append.datapackage(). However, since resource objects don't have a custom class, I had to implement a helper to append lists while retaining attributes. Some attributes can never be kept while appending, such as names (because they depend on the length of the object staying the same, which is not the case for append()). Things like dimnames would also break this custom append helper, as I only added an exception for names.

Furthermore, I re-added tests for the location of the $schema node in upgrade_resource() and upgrade_descriptor(). Both use the same append helper code.

TODO

  • Review by @peterdesmet (since he's not the original author)
  • Add test (any list)
  • Add package test (use check_package()
  • Add resource test (expect data_location and path to be retained)
  • Review by someone else to pass this
  • Rename helper to append_with_attributes()
  • See if 036bb52 is reverted. => yes, but separate test to check $schema is first property

Originally written by @PietrH in #362

Co-Authored-By: Pieter Huybrechts <48065851+PietrH@users.noreply.github.com>
@peterdesmet peterdesmet changed the title Create append helper function Create append helper function and set $schema as first attribute Aug 26, 2026
@codecov

codecov Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9506648) to head (953bc3c).

Additional details and impacted files
@@            Coverage Diff            @@
##                v2      #366   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines          792       807   +15     
=========================================
+ Hits           792       807   +15     

☔ View full report in Codecov by Harness.
📢 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.

@PietrH

PietrH commented Aug 28, 2026

Copy link
Copy Markdown
Member

I've added the requested tests, could you have another look @peterdesmet to see if I've covered everything you had in mind?

@peterdesmet
peterdesmet merged commit 27d932a into v2 Aug 29, 2026
9 checks passed
@peterdesmet
peterdesmet deleted the dollar-schema-first branch August 29, 2026 13:02
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.

Set $schema as first attribute

2 participants