Add generics for larrows, lpolygon, lrect and lsegments (fixes #29)

[courtiol]

I have now implemented the generics for larrows(), lpolygon(), lrect() and lsegments() as proposed in issue #29.

On my side, it passes all the R CMD checks (but I did not run your tests since they don't seem to involve these functions).

Open questions

• I have reordered the generics and methods by alphabetical order, which was perhaps a mistake... (happy to change that if needed).

• I updated the version number in DESCRIPTION, which I was not sure was needed or not.

• I added myself as "ctb" in DESCRIPTION too, but happy not to be as well (not sure what is your policy).

• I was not sure which arguments to keep in the generics.
  For now we have:

larrows(...)
llines(x, ...)
lpoints(x, ...)
lpolygon(x, ...)
lrect(...)
lsegments(...)
ltext(x, ...)

I did not put an argument (beyond the ellipsis) for larrows(), lrect()and lsegments() but we could alternatively defined them as:

larrows(x1, x2, ...) # or just x1?
lrect(xleft, ybottom, xright, ytop, ...) # or just xleft?
lsegments(x0, y0, x1, y1, x2, y2, ...) # or just x0?

I think it makes no difference and the logic of what to include or not in a generic is not clear to me.

++

[deepayan]

Thanks. I think the lack of an obvious argument on which to dispatch is the reason these were not generic to begin with. The versions in graphics (rect(), arrows(), etc.) are probably also not generics for the same reason.

This is territory I'm not very comfortable with, so it will need a little more thought. Dispatch will probably happen on the first argument regardless of whether it's named (at least that's how I read the R Language Definition). But I'm not sure of the pros and cons of having named arguments, although the current names (x0, x1, xleft) are too specific (and inconsistent), so their cons are obvious.

I think I will come back to this after the next release.

[courtiol]

Sad to hear that but I understand.

Sad because we need this to implement some change in terra (oscarperpinan/rastervis#101) to be used in another package (IsoriX).

From memory, the dispatch did work in my tests but I understand however the need to find a syntax that is well thought through so as to improve easy of use and both backward and forward compatibility.

If there is anyway I can help, please let me know.

[deepayan]

I'm not saying that this won't happen soon, just not in the next release, which will need to happen before R 4.3.0 is released because the current lattice is broken with 4.3.0.

I'm trying to get as many pending fixes in as I can, but this one will need a little more time. Once I get that out, I will be happy to get back to this.

[courtiol]

@deepayan, did you get a chance to look at this further?

With sp becoming obsolete as of next month (as far as I understand), it would be great to have a nice way to plot polygons on top of terra SpatRaster without involving sp, and this PR would make this possible via oscarperpinan/rastervis#101...

[deepayan]

Thanks for the reminder. I'm still a little worried that I don't fully understand how generic functions with no named arguments behave, but I can't see any obvious problems, other than that dispatch really happens on the first argument, irrespective of argument names. I'll go ahead and merge this soon.

[deepayan]

OK, I was waiting to push out a CRAN release with some minor changes, which is now done.

Could you update the DESCRIPTION and NEWS (which is now NEWS.md) in your PR, to make it cleanly mergeable?

[courtiol]

@deepayan: content should now be cleanly mergeable.