Skip to content

Removes unnecessary data.table call from as.data.table.array#7019

Merged
tdhock merged 15 commits intomasterfrom
faster-array
May 26, 2025
Merged

Removes unnecessary data.table call from as.data.table.array#7019
tdhock merged 15 commits intomasterfrom
faster-array

Conversation

@MichaelChirico
Copy link
Member

Re-push of the changes in #7010 (authored by @eliocamp) onto Rdatatable/data.table so that we get {atime} results directly here instead of waiting for follow-up.

@codecov
Copy link

codecov bot commented May 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.69%. Comparing base (73d79ed) to head (4a18f7f).
Report is 97 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7019   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files          79       79           
  Lines       14677    14678    +1     
=======================================
+ Hits        14486    14487    +1     
  Misses        191      191           

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

@tdhock
Copy link
Member

tdhock commented May 26, 2025

thanks! I added na.rm=FALSE to the atime test to avoid the error.
On my machine I see this result, which indicates a reduction in memory usage, but not much difference in time.
image

@github-actions
Copy link

github-actions bot commented May 26, 2025

Comparison Plot

Generated via commit 4a18f7f

Download link for the artifact containing the test results: ↓ atime-results.zip

Task Duration
R setup and installing dependencies 4 minutes and 21 seconds
Installing different package versions 8 minutes and 43 seconds
Running and plotting the test cases 2 minutes and 5 seconds

@tdhock
Copy link
Member

tdhock commented May 26, 2025

on CI we do see a small improvement in speed
image

@tdhock tdhock merged commit 2715663 into master May 26, 2025
11 checks passed
@MichaelChirico MichaelChirico added the atime Requests related to adding/improving/monitoring performance regression tests via atime. label Jul 8, 2025
@jangorecki jangorecki deleted the faster-array branch September 27, 2025 09:17
@MichaelChirico MichaelChirico restored the faster-array branch October 20, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

atime Requests related to adding/improving/monitoring performance regression tests via atime.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants