Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add run from spinup option #411

Merged
merged 8 commits into from Feb 6, 2018

Conversation

Projects
None yet
2 participants
@fmaussion
Copy link
Member

fmaussion commented Jan 31, 2018

  • Closes #394
  • Tests added/passed
  • Fully documented, including whats-new.rst for all changes
@fmaussion

This comment has been minimized.

Copy link
Member Author

fmaussion commented Jan 31, 2018

Still a WIP: this first commit simply refactors some functions

fmaussion added some commits Feb 2, 2018

@fmaussion

This comment has been minimized.

Copy link
Member Author

fmaussion commented Feb 2, 2018

OK, this should be it. CC @anoukvlug for a review.

Note that once again this is not backwards compatible (keyword names have changed). Altogether this is for a good purpose, the calls are now much simpler. Furthermore, no new task was needed, which is also nice.

@fmaussion fmaussion changed the title WIP: add run from spinup task Add run from spinup option Feb 2, 2018

@anoukvlug
Copy link
Contributor

anoukvlug left a comment

Thanks @fmaussion for making this nice addition! :D

bias=bias, seed=seed, filename=filename,
input_filesuffix=input_filesuffix)
bias=bias, seed=seed,
filename=climate_filename,

This comment has been minimized.

Copy link
@anoukvlug

anoukvlug Feb 5, 2018

Contributor

I am just wondering, why do you keep the "filename" keyword and not also change it to "climate_filename"?

@fmaussion

This comment has been minimized.

Copy link
Member Author

fmaussion commented Feb 5, 2018

I am just wondering, why do you keep the "filename" keyword and not also change it to "climate_filename"?

That's a very good point! The actual reason is that it would mean even more changes -- and I didn't want to do it all at once without a clear strategy. Will open an issue for this

@fmaussion fmaussion merged commit 22b3935 into OGGM:master Feb 6, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.2%) to 90.882%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.