Skip to content

Conversation

@suhaani-agarwal
Copy link
Contributor

This is in reference to issue #165 .
Replaced all given instances of plyr with data.table.
Please let me know if any changes are to be made

@tdhock
Copy link
Collaborator

tdhock commented Mar 10, 2025

this looks like a reasonable start.
let's see if it works...

@suhaani-agarwal
Copy link
Contributor Author

@tdhock I have removed dplyr arrange function and replaced it with data.table::setorder. Is this okay?

@suhaani-agarwal
Copy link
Contributor Author

@tdhock I have made the requested changes, please review and let me know if further changes are required.

@suhaani-agarwal
Copy link
Contributor Author

@tdhock is this ok? please let me know if more changes are required.

@suhaani-agarwal
Copy link
Contributor Author

I've made the requested changes. @tdhock

@tdhock
Copy link
Collaborator

tdhock commented Mar 15, 2025

please remove plyr from DESCRIPTION so we can see if checks pass ok

@suhaani-agarwal
Copy link
Contributor Author

please remove plyr from DESCRIPTION so we can see if checks pass ok

Done.

@suhaani-agarwal
Copy link
Contributor Author

@tdhock is there anything that needs to be done regarding this issue further?

@tdhock
Copy link
Collaborator

tdhock commented Mar 17, 2025

please click Resolve conversation for the items you believe are already fixed

@tdhock
Copy link
Collaborator

tdhock commented Mar 17, 2025

I'm seeing

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-renderer1-hjust-text-anchor.R:48:3'): (code run outside of `test_that()`) ──
Error in `.checkTypos(e, names_x)`: Object 'i' not found. Perhaps you intended [x, y, z]
Backtrace:
     ▆
  1. ├─...[] at test-renderer1-hjust-text-anchor.R:47:1
  2. ├─data.table:::`[.data.table`(...) at test-renderer1-hjust-text-anchor.R:47:1
  3. ├─.SD[iteration <= i] at test-renderer1-hjust-text-anchor.R:48:3
  4. └─data.table:::`[.data.table`(.SD, iteration <= i) at test-renderer1-hjust-text-anchor.R:48:3
  5.   └─base::tryCatch(...)
  6.     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
  7.       └─base (local) tryCatchOne(...)
  8.         └─value[[3L]](cond)
  9.           └─data.table:::.checkTypos(e, names_x)
 10.             └─data.table:::stopf(...)
 11.               └─data.table:::raise_condition(...)

@suhaani-agarwal suhaani-agarwal marked this pull request as draft March 18, 2025 07:37
@suhaani-agarwal
Copy link
Contributor Author

hi @tdhock I have resolved the error that was coming in renderer tests, but I am facing some errors in

helper-plot-data.r in the function cdata while replacing plyr with data.table
the error is as follows:
Error in ``[.data.table(df, scales == "x"): i evaluates to a logical vector length 12 but there are 2 rows. Recycling of logical i is no longer allowed as it hides more bugs than is worth the rare convenience. Explicitly use rep(...,length=.N) if you really need to recycle.

Can you please help me with correcting the code for helper-plot-data.r - cdata() function?

@suhaani-agarwal suhaani-agarwal marked this pull request as ready for review March 19, 2025 14:03
@suhaani-agarwal
Copy link
Contributor Author

hi @tdhock I have resolved the error that was coming in renderer tests, but I am facing some errors in

helper-plot-data.r in the function cdata while replacing plyr with data.table the error is as follows: Error in[.data.table ``(df, scales == "x"): i evaluates to a logical vector length 12 but there are 2 rows. Recycling of logical i is no longer allowed as it hides more bugs than is worth the rare convenience. Explicitly use rep(...,length=.N) if you really need to recycle.

Can you please help me with correcting the code for helper-plot-data.r - cdata() function?

hi @tdhock can you please reply to this query?

@tdhock
Copy link
Collaborator

tdhock commented Mar 22, 2025

I'm not sure

@suhaani-agarwal
Copy link
Contributor Author

The compiler tests are passing (except the gh permissions one because i have pushed via my fork not directly)
But I am not getting why the renderer tests are being stuck on the file "variable-value" . I run the tests at my end and the test is passing individually for variable-value but getting stuck when all renderer tests are run together.
This was not happening earlier (in the first few commits).

@suhaani-agarwal
Copy link
Contributor Author

I tested everything again in another branch and found out that the updated in the renderer test files might be the problem because without those updates and in different branch , all the tests are passing successfully (not getting stuck).
I will try creating a new PR with another branch for this and see if the tests pass there.

@suhaani-agarwal
Copy link
Contributor Author

suhaani-agarwal commented Apr 6, 2025

I have created a new PR for this, hence I am closing this one.

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.

2 participants