Skip to content

Conversation

oschulz
Copy link
Contributor

@oschulz oschulz commented Aug 20, 2025

I don't know what I was thinking in #1558, but Julia convention is of course
ArrayType{T}(undef, dims) and ArrayType{T}(undef, dims...), but not ArrayType(undef, T, dims).

Also, the typedef for ConcreteRArray was missing T and N parameters and the typedefs for ConcreteRNumber was missing T,

This PR fixes that (with deprecation for ConcreteRArray(undef, T, dims; kwargs...)) and adds tests.

Copy link
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

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

I wouldn't bother deprecating and just deleting the bad one here.

@oschulz
Copy link
Contributor Author

oschulz commented Aug 20, 2025

I wouldn't bother deprecating and just deleting the bad one here.

Ok, it's gone. :-)

1 similar comment
@oschulz
Copy link
Contributor Author

oschulz commented Aug 20, 2025

I wouldn't bother deprecating and just deleting the bad one here.

Ok, it's gone. :-)

Copy link

codecov bot commented Aug 20, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 43.44%. Comparing base (b6ffc96) to head (e6025e3).
⚠️ Report is 1422 commits behind head on main.

Files with missing lines Patch % Lines
src/ConcreteRArray.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1571       +/-   ##
===========================================
+ Coverage   21.66%   43.44%   +21.77%     
===========================================
  Files          46      124       +78     
  Lines        8048    22021    +13973     
===========================================
+ Hits         1744     9566     +7822     
- Misses       6304    12455     +6151     

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

@oschulz oschulz force-pushed the undef-ctors branch 2 times, most recently from 974bda9 to 36882a5 Compare August 20, 2025 20:45
@oschulz
Copy link
Contributor Author

oschulz commented Aug 21, 2025

Are the test failures related to this PR?

@giordano
Copy link
Member

No (and one is simply the coverage step failing to start at all)

@oschulz
Copy link
Contributor Author

oschulz commented Aug 21, 2025

No (and one is simply the coverage step failing to start at all)

Thanks. I'm good with this from my side then.

@wsmoses wsmoses merged commit 1d0c13f into EnzymeAD:main Aug 21, 2025
56 of 58 checks passed
@oschulz oschulz deleted the undef-ctors branch August 22, 2025 13:55
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