search

Como-DTC-Collaboration / como-models

Pedagogical materials for learning COVID-19 transmission dynamics modelling
https://Como-DTC-Collaboration.github.io/como-models/
MIT License
3 stars 0 forks source link

I16 urban vs rural communities #25

Closed solveigvandervegt closed 2 years ago

solveigvandervegt commented 3 years ago

pull request for the 2 communities model to be added to the como-models package.

Some issues remain around loading the excel files and I can therefor also not check if all the tests actually pass.

solveigvandervegt commented 3 years ago

@rcw5890 Thank you for going over this so carefully!!

solveigvandervegt commented 3 years ago

@rcw5890 thanks for taking another look! I fixed the matlib issue. I was using a function from that package but I found another way to do it, so the package has now been removed and the associated checks now pass. I am not sure what is going on with the error in ubuntu. I don't have a folder named Data, only data, which for as far as I can tell, is the preferred way of storing data in an R package. I thought that maybe the issue came from the fact that there is a folder named data and a file named data.R, but again, this is all following the guidelines for a proper R package. Since the other os don't complain, it must an Ubuntu issue

rccreswell commented 3 years ago

Hmm, the one you have on github does seem to have both Data and data. This is what I'm looking at: https://github.com/Como-DTC-Collaboration/como-models/tree/i16-urban-vs-rural-communities/data https://github.com/Como-DTC-Collaboration/como-models/tree/i16-urban-vs-rural-communities/Data

solveigvandervegt commented 3 years ago

hmmmm, that's really weird cause on my local machine, I don't, so I am not sure where the Data folder is coming from!

solveigvandervegt commented 3 years ago

For some reason, it appears that once I push to the origin, it is splitting all the data files into two folders. Let me see if I can change some things around

solveigvandervegt commented 3 years ago

all fixed :)

rccreswell commented 3 years ago

all fixed :)

Thanks for making all those changes and getting the tests passing. This is looking great but I did have a couple of other various thoughts, not sure if github is hiding them?

solveigvandervegt commented 3 years ago

all fixed :)

Thanks for making all those changes and getting the tests passing. This is looking great but I did have a couple of other various thoughts, not sure if github is hiding them?

Ah, yeah I think I just saw them! I will make the changes tomorrow!

solveigvandervegt commented 3 years ago

I have made the changes you requested. There are some conditions on inputs, but hopefully those are explained sufficiently in the documentation and the vignette. I am not sure what is going on with Windows, sometimes I run the checks and it is fine, and sometimes it is not.

solveigvandervegt commented 3 years ago

I think it is ready, but let me know what y'all think!

solveigvandervegt commented 3 years ago

@rcw5890 Great, thank you so much for all your help Richard!!