Support custom types#222
Conversation
- exception for date-alike classes
- fixes labelling issue for dates etc. when flipped
- add xmin and xmax columns to upstream datapoints objects
This comment was marked as resolved.
This comment was marked as resolved.
|
Converting to draft so I can move this closer to the ultimate goal before merging. |
|
@grantmcdermott issues all resolved (I think). Happy to make more changes as you think is appropriate. |
grantmcdermott
left a comment
grantmcdermott
left a comment
There was a problem hiding this comment.
Excellent, thanks @vincentarelbundock. I think we're close now. I left a few comments that I missed first time around and also have two meta requests:
- Can you please add some snapshot tests for the new model prediction types (lm/glm/loess)?
- Can you please add a NEWS item. Don't forget to credit yourself!
grantmcdermott
left a comment
grantmcdermott
left a comment
There was a problem hiding this comment.
Nearly there. Only a few more comments. Thanks for bearing with me @vincentarelbundock.
|
All good suggestions. Resolved all, but the formula conversion. Agree but have other things to attend now. Might circle back eventually if I think about it. |
|
Brilliant. I'm happy to merge now, but will change the PR title first if you don't mind (to something more explicit given the expanded scope of the PR). |
grantmcdermott
left a comment
grantmcdermott
left a comment
There was a problem hiding this comment.
Outstanding @vincentarelbundock! Thanks so much for putting in the effort, as well as the careful design consideration.
Really, really stoked with this functionality.
3 participants
This is the last preparatory step in my master plan to allow custom user-supplied geoms.
Split
draw_elements()into a separate drawing function for each geom, such asdraw_polygon(),draw_ribbon(), etc. Then,draw_elements()uses aswitch()call to select the correct one based on the value oftype.I wrote this from a fork of your
flipPR, so it can supplant it.Sorry, I should have waited for a merge, but the family went away for a thing and I found myself with a couple spare hours.
GM edit/update for tracking purposes:
Closes #226, closes #227, closes #228, closes #229.