Skip to content
This repository was archived by the owner on Jan 20, 2025. It is now read-only.

Add missing DefaultArrayInterface methods #22

Closed
wants to merge 1 commit into from

Conversation

lkdvos
Copy link

@lkdvos lkdvos commented Jan 9, 2025

This PR adds some of the methods for DefaultArrayInterface that were erroring because there was no AbstractArrayInterface fallback.

I'm still not sure if this needs to be implemented with Base.invoke or not, but the tests are not complaining?

@mtfishman
Copy link
Member

Great, thanks!

I'm still not sure if this needs to be implemented with Base.invoke or not, but the tests are not complaining?

That's good, probably we should avoid using invoke if we can so let's try this out and see how it goes.

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 38.85%. Comparing base (d194ece) to head (8d0ecd4).

Files with missing lines Patch % Lines
src/defaultarrayinterface.jl 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
- Coverage   39.86%   38.85%   -1.02%     
==========================================
  Files           9        9              
  Lines         306      314       +8     
==========================================
  Hits          122      122              
- Misses        184      192       +8     
Flag Coverage Δ
docs 37.69% <0.00%> (-0.99%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@mtfishman
Copy link
Member

Can you add some tests? I know I haven't been great about testing this package but there is no time like the present to start!

@mtfishman
Copy link
Member

@lkdvos can you move this PR over to https://github.com/ITensor/DerivableInterfaces.jl?

@lkdvos lkdvos closed this by deleting the head repository Jan 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants