spatialmodel / inmap

InMAP reduced-form air quality model for fine particulate matter (PM2.5)
GNU General Public License v3.0
59 stars 42 forks source link

preproc: Add multi-hour GEOS-Chem data #42

Closed SumilThakr closed 6 years ago

SumilThakr commented 6 years ago

Please take a look.

ctessum commented 6 years ago

It looks like a couple of files got added by accident. To remove them you can do

git rm .geoschem.go.swp inmap/testdata/testSR.ncf

ctessum commented 6 years ago

Additionally, because I made some changes to the master branch that included moving the location of the configuration file, you will also have to do:

git mv inmap/configExampleGEOSChem.toml cmd/inmap/configExampleGEOSChem.toml

SumilThakr commented 6 years ago

I've made the changes you suggested and committed them to my chemhours branch. I've also run all the tests and they are all okay. N.B. When I click on the Pull request tab, GitHub says: "Can’t automatically merge. Don’t worry, you can still create the pull request." I think this has something to do with changing the directory of the config file?

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.05%) to 67.976% when pulling 69f60e22c58d4da2db1d209e9ea18ff097915ae7 on SumilThakr:chemhours into c78088e93f12bafb55f5379f051cd05dc4527805 on spatialmodel:master.

ctessum commented 6 years ago

Hi Sumil, sorry for the delay in getting to this. I think I did something that didn't help the situation but it should still be fixable.

The first thing to do is you squash all of your commits into one. You can do this using

git checkout chemhours
git rebase -i HEAD~9

and then follow the instructions. Here's a blog post about it: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

After this, you will will need to rebase your changes on the master branch so that your changes are being made on top of the most recent version of master rather than a previous version:

git checkout master
git pull https://github.com/spatialmodel/inmap.git master
git checkout chemhours
git rebase master

This may work without a problem, or there may be conflicting changes that you will need to resolve before the rebase can finish. This can be a little tricky but there should be internet guides out there, or let me know if you have any questions.

Then you will need to force-push your changes using git push -f origin chemhours. After all this you should end up with one commit in the pull request, which we can then merge in to the master branch.

SumilThakr commented 6 years ago

It looks like this worked ... the branch "chemhours" says This branch is 1 commit ahead of spatialmodel:master.

On Fri, Jun 15, 2018 at 3:55 AM, Chris Tessum notifications@github.com wrote:

Hi Sumil, sorry for the delay in getting to this. I think I did something that didn't help the situation but it should still be fixable.

The first thing to do is you squash all of your commits into one. You can do this using

git checkout chemhours git rebase -i HEAD~9

and then follow the instructions. Here's a blog post about it: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

After this, you will will need to rebase your changes on the master branch so that your changes are being made on top of the most recent version of master rather than a previous version:

git checkout master git pull https://github.com/spatialmodel/inmap.git master git checkout chemhours git rebase master

This make work without a problem, or there may be conflicting changes that you will need to resolve before the rebase can finish. This can be a little tricky but there should be internet guides out there, or let me know if you have any questions.

Then you will need to force-push your changes using git push -f origin chemhours. After all this you should end up with one commit in the pull request, which we can then merge in to the master branch.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spatialmodel/inmap/pull/42#issuecomment-397498617, or mute the thread https://github.com/notifications/unsubscribe-auth/AbgWXSqvOLwMH7ksXRppWMhFdDNtz_wSks5t8yIlgaJpZM4UCLPx .

ctessum commented 6 years ago

Looks good. If you fix the issue I point out above and squash it back into one commit, hopefully the tests will pass and then we can merge it with the master branch.

SumilThakr commented 6 years ago

Done! The tests fail on my end, but I think it's because there are a lot of new libraries to install ... doing that now

On Mon, Jun 18, 2018 at 4:57 PM, Chris Tessum notifications@github.com wrote:

Looks good. If you fix the issue I point out above and squash it back into one commit, hopefully the tests will pass and then we can merge it with the master branch.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spatialmodel/inmap/pull/42#issuecomment-398104392, or mute the thread https://github.com/notifications/unsubscribe-auth/AbgWXSYsQyagtrnjNFC6j78N93TlhszYks5t983tgaJpZM4UCLPx .

ctessum commented 6 years ago

Looks good, thanks!