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

Added task to get the pointwise catchment area and linear mass balance #725

Merged
merged 7 commits into from Apr 30, 2019

Conversation

Projects
None yet
3 participants
@MatCast
Copy link
Contributor

commented Mar 27, 2019

  • Tests added/passed
  • Fully documented
  • Entry in whats-new.rst
@fmaussion

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

Thanks! This is looking good. I'll have a more detailed look next week.

@fmaussion

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

@MatCast this is very good thanks!

in terms of API this is not exactly what I was looking for - I want to hide this under a more general task. If you agree I'll add some changes on top of your PR, ok?

@MatCast

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

@fmaussion of course go ahead please.

@MatCast

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

I found a bug in the code which would generate a ValueError for some glaciers. I fixed it but let me merge the updated code before merging this into master. Or should i merge it with the corrected API that you had in mind as well?

@fmaussion fmaussion force-pushed the MatCast:master branch from 61ee554 to b28eaab Apr 25, 2019

@pep8speaks

This comment has been minimized.

Copy link

commented Apr 25, 2019

Hello @MatCast! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-04-30 14:13:55 UTC
@fmaussion

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

@MatCast sorry I missed your previous posts. I think I've changed too much now, and likely my version is buggy as well so I'll try to test it on more glaciers.

I'm thinking a bit bigger now because I also have interest to get more things related to ice thickness in OGGM, I'll add some documentation about this first, then continue in another PR.

@fmaussion fmaussion force-pushed the MatCast:master branch from 8d2e869 to 37b789d Apr 30, 2019

@fmaussion fmaussion merged commit 7349369 into OGGM:master Apr 30, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 87.592%
Details
@fmaussion

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

Thanks @MatCast !

This was referenced Apr 30, 2019

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.