clustRviz
clustRviz copied to clipboard
DataSlingers
M farouk b patch 3
Hi @MFaroukB, as I commented inline, there are some changes re: default values and documentations that should be fixed before we can merge this.
They should be minor (I hope!) but feel free to ping me with questions.
- M
Yes, I will get them done in a second.
I have just been working on the application and didn't see the reply the moment it was sent.
On Mon, Apr 8, 2019, 21:05 Michael Weylandt [email protected] wrote:
Hi @MFaroukB https://github.com/MFaroukB, as I commented inline, there are some changes re: default values and documentations that should be fixed before we can merge this.
They should be minor (I hope!) but feel free to ping me with questions.
- M
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DataSlingers/clustRviz/pull/90#issuecomment-481072942, or mute the thread https://github.com/notifications/unsubscribe-auth/AvEP3T6ioFtl87fpY_g0h8ACSaEAsxfTks5ve_V8gaJpZM4cjS5R .
The Only problem is the function animate is not even used in saveviz.CBASS
On Mon, Apr 8, 2019, 21:37 Mahmoud Badawi [email protected] wrote:
Yes, I will get them done in a second.
I have just been working on the application and didn't see the reply the moment it was sent.
On Mon, Apr 8, 2019, 21:05 Michael Weylandt [email protected] wrote:
Hi @MFaroukB https://github.com/MFaroukB, as I commented inline, there are some changes re: default values and documentations that should be fixed before we can merge this.
They should be minor (I hope!) but feel free to ping me with questions.
- M
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DataSlingers/clustRviz/pull/90#issuecomment-481072942, or mute the thread https://github.com/notifications/unsubscribe-auth/AvEP3T6ioFtl87fpY_g0h8ACSaEAsxfTks5ve_V8gaJpZM4cjS5R .
I have added 4 commits to my previous patch. I believe it should appear now for you!
If it didn't, let me know. I will redo it as I did before.
On Mon, Apr 8, 2019, 21:39 Mahmoud Badawi [email protected] wrote:
The Only problem is the function animate is not even used in saveviz.CBASS
On Mon, Apr 8, 2019, 21:37 Mahmoud Badawi [email protected] wrote:
Yes, I will get them done in a second.
I have just been working on the application and didn't see the reply the moment it was sent.
On Mon, Apr 8, 2019, 21:05 Michael Weylandt [email protected] wrote:
Hi @MFaroukB https://github.com/MFaroukB, as I commented inline, there are some changes re: default values and documentations that should be fixed before we can merge this.
They should be minor (I hope!) but feel free to ping me with questions.
- M
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DataSlingers/clustRviz/pull/90#issuecomment-481072942, or mute the thread https://github.com/notifications/unsubscribe-auth/AvEP3T6ioFtl87fpY_g0h8ACSaEAsxfTks5ve_V8gaJpZM4cjS5R .
This looks good for the changes for the gganimate based plots (saveviz.CARP with type = "path" and dynamic = TRUE) -- did you want to make changes for the animation::saveGIF based plots also or no?