Update tests + various lints

[olivroy]

• Update tests to use more expect_equal(object, expected), (part of 396 R6 impl #471)
• More use of snake case in tests to help finalize transition. It seems to have worked without issues for many months now, congrats on the seamless transition to snake_case.
• Use of "" instead of ''
• Other lints flagged to me by lintr.

[JanMarvin]

Thank you! There were some changes that really made me smile.

I’m not sure about lengths() on lists. This might have been from R 4.0 and even though that’s now a long time ago, my employers IT department is very slow with non crucial software updates (we use SAS) and I had to run openxlsx2 on R 3.6 iirc just last year. Therefore please have a look.

I’d also like to see one or two tests that still use camel case while we still support it. Otherwise something might break unknowingly to us.

And, if I ever again see a PR with your name and the number of impacted files is larger than 5 and it’s not a roxygen update I seriously will close it without any hesitation 😜

[olivroy]

Sorry, sometimes, I don't know where I am going and it is easy to identify things when scanning code and you know you're never gonna come back to it if you don't touch it immediately.

Let's hold off test changes for snake_case.

[JanMarvin]

It’s alright, I get that, but I still don’t like these hundreds of lines all over the place PR. But I trust your judgment, if you say these lines are justified, that’s almost enough reason for me to merge. I just wanted the lengths thing checked and one or two tests for the camel case arguments.

[olivroy]

It’s alright, I get that, but I still don’t like these hundreds of lines all over the place PR. But I trust your judgment, if you say these lines are justified, that’s almost enough reason for me to merge. I just wanted the lengths thing checked and one or two tests for the camel case arguments.

I'd rather have the smaller PRs merged first, then I will merge the conflicts and see if I still think this is worth it.

[JanMarvin]

Very well. Now that the lengths() problem is solved, I have no real objections. I'll still write expect_equal(exp, got), but feel free to merge. Also you haven't swapped it everywhere :)

Also, I'm going to do you a favor: I plan to add a na.strings - > na_strings conversion before the next release. Let me know if you want to tackle this, otherwise I'll have a look.

Thank you so much for your work! I really appreciate it, even though it must feel like trying to clean up a petulant child's room. Hopefully it's for a greater whole!

[olivroy]

Very well. Now that the lengths() problem is solved, I have no real objections.

lengths() seems to have appeared in R 3.2 however, but better safe than sorry.

I'll still write expect_equal(exp, got), but feel free to merge. Also you haven't swapped it everywhere :)

No problems!

Also, I'm going to do you a favor: I plan to add a na.strings - > na_strings conversion before the next release. Let me know if you want to tackle this, otherwise I'll have a look.

Are you sure if this is worth the effort? Maybe num_fmt vs numfmt is the one I forget the most at this point.

Thank you so much for your work! I really appreciate it, even though it must feel like trying to clean up a petulant child's room. Hopefully it's for a greater whole!

Well, on my own, I always write terrible code, and I hate cleaning after myself. i hope that cleaning after others will help me write cleaner code at first? maybe that is delusion.

I will merge with your blessing then!

[JanMarvin]

Regarding the length stuff: Well they say time flies when you’re having fun 😉 , but actually I was thinking about a behavior change in length which is why I coded it the way it is. Something like this in 4.2.3.

As "POSIXlt" objects may be “partially filled” and their list components meant to be recycled, length() now is the length of the longest component.

[JanMarvin]

Also, I'm going to do you a favor: I plan to add a na.strings - > na_strings conversion before the next release. Let me know if you want to tackle this, otherwise I'll have a look.

Are you sure if this is worth the effort? Maybe num_fmt vs numfmt is the one I forget the most at this point.

I intend to add something like gsub(".", "_", arguments) to standardization() and switch the default to na_strings. Since we've switched to snake case everywhere it began to feel strange.

Have typed numfmt so often, that I do not stumble on this one anymore. But there are some cases where we have these inconsistencies. Still I'm quite happy with the stability of the API. Would have thought that it would be noisier.

[olivroy]

Yeah, me too, that's why I'd rather have numfmt everywhere, but not too important I guess!