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

MAINT: Update coveragerc #1050

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adler-j
Copy link
Member

@adler-j adler-j commented Jun 29, 2017

This file has obviously become way outdated

Copy link
Member

@kohr-h kohr-h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

@kohr-h
Copy link
Member

kohr-h commented Jun 29, 2017

Hm, did you check where this decrease of coverage comes from? Seems like some stuff is omitted that shouldn't.

@adler-j
Copy link
Member Author

adler-j commented Jun 30, 2017

I agree, something definitely went wrong here. I'll look through it

@kohr-h kohr-h added this to Needs review in PR Status Oct 1, 2017
@kohr-h kohr-h moved this from Needs review to Needs improvement in PR Status Oct 2, 2017
@kohr-h
Copy link
Member

kohr-h commented Nov 14, 2017

Status?

@adler-j
Copy link
Member Author

adler-j commented Nov 14, 2017

I'll merge this for now, but it seems our coverage is lower than we thought :/

@kohr-h
Copy link
Member

kohr-h commented Nov 14, 2017

Please remove the "merge master into branch" commit.

@@ -10,16 +10,19 @@ omit =
odl/diagnostics/*
odl/util/*

# Omit until coveralls supports cuda.
odl/space/cu_ntuples.py

# Omit until coveralls supports pywt.
odl/trafos/wavelet.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We install pywavelets on Travis now, so this line can be removed.

@@ -46,6 +49,13 @@ exclude_lines =
# Decorators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment here: return NotImplemented should go off the list, since it's part of actual functionality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead, we may think about adding the RuntimeError stuff that's just defensive coding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
PR Status
In revision
Development

Successfully merging this pull request may close these issues.

None yet

2 participants