Fixes to the recent pr - Githubissues

asiripanich / emdash

An e-mission deployer's dashboard. See https://github.com/e-mission/e-mission-docs.

https://emdash.amarin.dev

Other

6 stars 3 forks source link

Fixes to the recent pr #14

Closed shankari closed 3 years ago

shankari commented 3 years ago

Two fixes:

Bump up the dplyr version because otherwise the code fails
Have the input options set using environment variables so that it is easier to include in a docker-compose
- We can also then publish an image

@asiripanich any thoughts on publishing an image? It takes so long to build the image, that I think it would be useful to publish an image on dockerhub that at least includes all the R dependencies.

Users can then pass in environment variables, customize the Dockerfile further, or mount a volume to run from locally checked out source.

Do you want to publish the image or should I?

asiripanich commented 3 years ago

@shankari, I just tested your PR. Is this how the trips for this user should look like?

Master	This PR

asiripanich commented 3 years ago

Two fixes:
* Bump up the dplyr version because otherwise the code fails

* Have the input options set using environment variables so that it is easier to include in a docker-compose

  * We can also then publish an image
@asiripanich any thoughts on publishing an image? It takes so long to build the image, that I think it would be useful to publish an image on dockerhub that at least includes all the R dependencies.

Users can then pass in environment variables, customize the Dockerfile further, or mount a volume to run from locally checked out source.

Do you want to publish the image or should I?

wrt. publishing a docker image for emdash, I agree that we should do that #9. Let me see if there is a GHA workflow somewhere in the R community that we can use.

ahcyip commented 3 years ago

@shankari will be able to better explain if those trips should be in or out.

I now understand that it might work better if start_ts, end_ts, and ts (numerics rather than datetimes) were used for the trip-location join (to be more exact and to avoid time zone conversion issues). but the ts variables are cleaned out in the tidy_trips and tidy_locations.

I don't have time to work on this though.

asiripanich commented 3 years ago

@shankari I'm happy to fix this issue if you can let me know how they should look like.

shankari commented 3 years ago

@asiripanich I haven't had time to set up my emdash environment since then because I was working on other tasks. I should be getting back to emdash today/early next week, and can let you know then. I will also be looking into a generic module for user inputs, so will file an issue for the design decision

shankari commented 3 years ago

@asiripanich this is what the trips look like for me with the fixes. So generally the same as yours except for the horizontal line.

Is the horizontal line real or an artifact? What does it represent?

asiripanich commented 3 years ago

@asiripanich this is what the trips look like for me with the fixes. So generally the same as yours except for the horizontal line.

Is the horizontal line real or an artifact? What does it represent?

Yes that is an artifact. So shouldn't all the lines be connected? They are trajectories of the same user right?

shankari commented 3 years ago

Yes that is an artifact. So shouldn't all the lines be connected? They are trajectories of the same user right?

They are all trajectories of the same user (me), but I pulled only selected dates, that had particularly tricky travel patterns, for the unit tests. And I created the test mongodb from the unit tests. So it is expected that the trips are disconnected.