Fix #7354, Implementing NSE in cube

[sisyphuswastaken]

Pertains to issue #7354.
Changes implemented:

• isolated logic for processing SDcols from the [.data.table function, encapsulated it into a helper function .processSDcols.
• Added tests for code coverage
• removed the try-catch error block, to make errors more specific and debugging easier
  This PR continues work from Implementing NSE in cube #7540 and and Implementing NSE in cube #7543.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.85%. Comparing base (d65e46f) to head (99363e1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7603   +/-   ##
=======================================
  Coverage   98.85%   98.85%           
=======================================
  Files          87       87           
  Lines       17128    17192   +64     
=======================================
+ Hits        16932    16996   +64     
  Misses        196      196           

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

[sisyphuswastaken]

I've added new tests for code coverage, and tried to undo the unnecessary changes that occurred when I had replaced the full function while integrating .processSDcols.

[sisyphuswastaken]

Hello! Bumping on this, let me know if any changes are to be made to the PR, I'd be happy to adjust :) @jangorecki

[sisyphuswastaken]

sorry for the delays, I was swamped with assignments but I'll push changes within this week. Also since I made a new branch, it seems as if some of my changes (the use of the helper function in [.data.table) weren't transferred properly. I'll fix that and have the changes ready for review within this week. I apologize for the inconvenience caused.

[sisyphuswastaken]

I've included the helper in [.data.table and added tests. Some might seem redundant since I added them for code coverage, which I'm still working on. I'm not yet sure how to tackle the project’s coverage checks failing, but I'm actively working on it.

[sisyphuswastaken]

I've implemented the changes as per the previous review. I've added tests for code coverage and integrated the helper

Request has been addressed, although I cannot now re-review to approve, therefore dismissing previous request for change

[sisyphuswastaken]

@jangorecki
Following up on my PR, let me know if any changes are to be made :)
Thank you!

[ben-schwen]

Why dont we need eval(sub.result, enclos, enclos) as in the .SD branch from [?

[ben-schwen]

To be quite frank, I would be strongly in favor of modularizing the code moloch we currently have in the if (any(c(".SD","eval","get","mget") %chin% av)) branch and use it here rather than bringing in this hard to grasp piece a 2nd time.

[sisyphuswastaken]

Hi @ben-schwen, thank you for the review. I've noted both points and will try to address them as soon as possible. I'm currently tied up with some work on my end but will get back to this asap. That said, if this is blocking anything, do let me know and I'll prioritise accordingly. thank you for the patience!