TST: restructure internal extension arrays tests (split between /arrays and /extension) #22026

jorisvandenbossche · 2018-07-23T14:59:48Z

Pull request to discuss what to do with the tests for internal EAs (and one of the comments I still had in #21160)

Basically, I would keep the tests/extension/.. only for subclassing the base extension array test suite, and any array-specific functionality is tested in tests/arrays/.. (eg closed attribute for IntervalArray, specific arithmetic behaviour for IntegerArray, ...)

This means that when adding a test related to EAs, we need to think about: is this testing something that is applicable to all EAs? (-> add a base test to tests/extension/base so this is tested for all internal and external EAs) or is this testing something specific to a particular EA? (-> add a test in tests/array/EAtype/..)
Of course often there can be some ambiguity here.

Main reason that I would split them is that over time, we probably add a lot of EA-type-specific tests, and then keeping the general ones mixed with the specific ones will make it only confusing / hard to see what is going on. Drawback is of course that it is tested in two places.
In practice what I propose in this PR, is also what we already do for Categorical at the moment: Categorical has its own tests in tests/arrays/categorical (and probably also some in indexes and frame, ..), but we also run the base extension tests for Categorical in tests/extension/

…/integer

jorisvandenbossche · 2018-07-23T15:03:30Z

pandas/tests/arrays/test_integer.py

+            assert 'length' in result
+
+
+class TestConstructors(BaseInteger):


There is still some dependence on the base extension tests here, in the fact that we use the overridden self.assert_series_equal etc.
However, I think that tm.assert_series_equal should be able to correctly handle our internal EAs, and this should not be necessary.

And apparently the tm. methods already work, so not sure why they were overridden in original test_integer.py (@jreback do you remember?)

pandas/pandas/tests/extension/integer/test_integer.py

Lines 92 to 110 in a439472

def assert_index_equal(self, left, right, *args, **kwargs):

left_na = left.isna()

right_na = right.isna()

tm.assert_numpy_array_equal(left_na, right_na)

return tm.assert_index_equal(left[~left_na],

right[~right_na],

*args, **kwargs)

def assert_series_equal(self, left, right, *args, **kwargs):

left_na = left.isna()

right_na = right.isna()

tm.assert_series_equal(left_na, right_na)

return tm.assert_series_equal(left[~left_na],

right[~right_na],

*args, **kwargs)

jorisvandenbossche · 2018-07-23T15:06:48Z

pandas/tests/arrays/test_integer.py

+        self.assert_series_equal(result, expected)
+
+
+class TestArithmeticOps(BaseOpsUtil, BaseInteger):


BaseOpsUtil here is only used for get_op_from_name. So probably makes sense to simply copy that, or have somewhere a utility function for that.

jorisvandenbossche · 2018-07-23T15:08:26Z

pandas/tests/extension/integer/test_integer.py

+                    result = result.astype(float)
+            elif op_name.startswith('__r'):
+                # TODO reverse operators result in object dtype
+                # see https://github.com/pandas-dev/pandas/issues/22024


some of the added checks here have to do with certain 'buggy' (or unexpected) behaviours (see the issues I opened)

if you are doing a PR to move things, don't change code. I have no idea what you are changing here.

Well, the original code is still in tests/arrays/test_integer.py, so I only moved it there.
The changes here are simply to get the base class tests working instead of overwriting them as you did (and again, those custom tests are still in the other file).

jorisvandenbossche · 2018-07-23T15:09:57Z

General note: I didn't really think about how to structure the new arrays/test_integer.py and test_interval.py files, for now simply copied things from the extension tests. In the future we should restructure that (and if there will be added more, probably makes sense to have a subdir for each array type with multiple files)

TomAugspurger · 2018-07-23T19:55:26Z

+1 to your proposed restructure.

codecov · 2018-07-24T08:38:53Z

Codecov Report

Merging #22026 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #22026   +/-   ##
=======================================
  Coverage   92.04%   92.04%           
=======================================
  Files         169      169           
  Lines       50782    50782           
=======================================
  Hits        46742    46742           
  Misses       4040     4040

Flag	Coverage Δ
#multiple	`90.45% <ø> (ø)`	⬆️
#single	`42.3% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2e1a10...db2836c. Read the comment docs.

jorisvandenbossche · 2018-07-25T09:13:22Z

@jreback @jbrockmendel @jschendel any comments?

jreback · 2018-07-25T09:20:23Z

I think this is terribly confusing

where do so yes something is in 2 places

I would much be all internal arrays in there entirely to pandas/tests/arrays

and leave the base classes and inherited machinery in pandas/tests/extension

jorisvandenbossche · 2018-07-25T09:26:04Z

I would much be all internal arrays in there entirely to pandas/tests/arrays
and leave the base classes and inherited machinery in pandas/tests/extension

So you mean leaving the actual test base classes in pandas/test/extension but inheriting from them and overwriting certain tests in pandas/tests/array? How is that not more confusing to what I proposed?

jreback · 2018-07-25T09:58:44Z

So you mean leaving the actual test base classes in pandas/test/extension but inheriting from them and overwriting certain tests in pandas/tests/array? How is that not more confusing to what I proposed?

I would simply move test_integer, test_interval, and test_categorical in there entirety. All the tests are then in 1 place. It is immateterial that a test is inherited or not. In fact I didnt' respond to your comments above, but that IS the reason I purposely write the tests in the way I did. When debugging you want to change a single line and not even think / worry about any testing artifices (e.g. the fact that most tests are automatically run on your behalf and thus inherited).

jreback

see comments

jorisvandenbossche · 2018-07-25T10:17:58Z

Sorry, Jeff, I don't understand your comment. Can you try to clarify it?

It is immateterial that a test is inherited or not.

What does this mean? You mean that you don't find it relevant or confusing that tests in pandas/tests/array are inheriting from tests in pandas/test/extension ?

In fact I didnt' respond to your comments above, but that IS the reason I purposely write the tests in the way I did.

You mean the fact that get_op_from_name is used in both places (my comment #22026 (comment))? That is currently the only duplication (10 lines of code), so I don't think this is any blocker for any decision, we can easily create a common utility function for that if needed.

When debugging you want to change a single line and not even think / worry about any testing artifices.

Can you give a concrete example or clarify this?
How is having base tests in one directory and the inherited tests / overwritten methods in another directory helping with this? That would exactly mean that changing a line in one place can have consequences for tests located somewhere else, which makes it IMO worse.

jorisvandenbossche · 2018-07-25T10:21:21Z

To be clear: my proposal is to keep only the most basic tests as possible in pandas/tests/extension, so eg only test there that the Base extension tests actually make sense using the internal extension arrays, not to have it as exhaustive tests of the internal extension array.
I could move pandas/tests/extension/integer/test_integer.py to for example pandas/tests/array/integer/test_base_extension.py, but still keeping the 'real' integer array tests separate from the inherited extension base test class tests. In such a case I only don't see the added value to move them over there.

jreback · 2018-07-25T10:49:52Z

well that’s the issue

i couldn’t find an obvious split which makes it clear of what belongs where

jorisvandenbossche · 2018-07-25T10:59:36Z

i couldn’t find an obvious split which makes it clear of what belongs where

Well, I propose here a possible, IMO relatively clear, split

TomAugspurger · 2018-07-25T11:06:00Z

Well, I propose here a possible, IMO relatively clear, split

Agreed. When I'm working on the interface, I want to be able to run it against all the test and public EAs, so I'd like to run pytest pandas/tests/extensions. There's no need to run EA-specific things like the Categorical constructor tests when working on the EA interface.

jreback · 2018-07-25T11:23:50Z

this is not clear at all
i don’t think it adds value
rather it makes the question of where to put things come up

TomAugspurger · 2018-07-25T11:42:33Z

Interface test under extensions. Other tests under arrays.

…

________________________________ From: Jeff Reback <notifications@github.com> Sent: Wednesday, July 25, 2018 6:23:55 AM To: pandas-dev/pandas Cc: Tom Augspurger; Comment Subject: Re: [pandas-dev/pandas] TST: restructure internal extension arrays tests (split between /arrays and /extension) (#22026) this is not clear at all i don’t think it adds value rather it makes the question of where to put things come up — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#22026 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIkGa4t9zZ6i4DTSbajxHWk5PZlF1ks5uKFVLgaJpZM4VbJ4J>.

jorisvandenbossche · 2018-07-25T11:56:52Z

The current organisation is for sure not clear as well.

For example, when actually doing the reorganisation in this PR, I had to look at lot which tests were in the base classes that were or were not overridden, which made it very complicated to work with those tests. If, eg in the TestArithmeticOps of the integer array tests, you need to override almost everything from the base class, there is something wrong (and in fact, it was also not needed as I show in this PR).

Also, for the integer array, we are going to add a lot more tests while we develop it further (proper reduction, groupby, overflow handling, ...) that all will have a lot of integer-array specificities. Further mixing tests for this with Base test classes that are not meant for such specificities, will further complicate this IMO.

As Tom summarized, the proposed split: in the extension tests we only run the existing interface tests (the base classes) and only do the minimum to get those passing for eg IntegerArray (see this PR), everything else that is specific about integer arrays is tested in tests/arrays/integer.

So if you really think that this is "terribly confusing", then please try to give an alternative proposal.

jreback · 2018-07-25T12:01:25Z

@jorisvandenbossche its up to you to give a new proposal. I think this confusing. If I am confused, think about the average contributor.

TomAugspurger · 2018-07-25T12:05:38Z

its up to you to give a new proposal.

He has, with this PR!

Are you -1 here? Otherwise I don't think it's worth trying to explain again.

jreback · 2018-07-25T12:06:25Z

yes I am -1 here. I think the existing test structure is fine.

jreback · 2018-08-21T13:25:16Z

Then why do you bring up categorical as an example structure? It's split by interface vs. custom.

I am bringing it up as an example of the functionaility split. To be honest its VERY VERY hard to test what the actual custom type vs specific test is doing.

This is my point. We need to split by functionaility FIRST. and ONLY. This custom type / interface splitting is very confusing.

jreback · 2018-08-21T13:25:37Z

so I am done for today with this. Merge what you want.

jorisvandenbossche · 2018-08-21T13:26:18Z

maybe this is true, but it is totally non-obvious which ones are which

Again, as said multiple times above, we already have the same situation currently in master, where you also need to decide whether to put something in the BaseExtension classes or as a new test function/method in the same file. If you keep saying that it is not obvious how to split them, can you then at least answer to this?

becuase they are all agglomerated into a single file. if you would split them up I would be more ameanable here.

Euhm, in master all tests are agglomareated into a single file test_integer.py, I split them here.
But in case you mean all interface tests being agglomerated into a single file (so indeed combining eg groupby and indexing interface tests into a single file): the current extension/test_integer.py file with all interface tests in this PR is 250 lines long, do you really want to split that in 11 files? (that's in how many files the BaseExtension tests are splitted, nicely splitted by functionality)

jorisvandenbossche · 2018-08-21T13:27:46Z

Jeff, to come back to splitting up by functionality (#22026 (comment)). In practice (over time), we will not have

pandas/tests/extension/
    /integer/
        test_integer_extension_interface.py
        test_integer_custom.py

but of course something like:

    /integer/
        test_integer_extension_interface.py
        test_constructor.py
        test_dtypes.py
        test_casting.py
        test_ops.py
        ....

So we will split by functionality for the custom tests. We only propose to keep the basic interface tests together in a single file (not all the other tests), as they are simply inheriting from the BaseExtension tests.

jorisvandenbossche · 2018-08-21T15:28:02Z

Added a commit that should hopefully fix the tests. One part of it splitted off as its own bug fix: #22441

It would be good if some people could review the actual code changes.

pandas/tests/extension/test_integer.py

…-testing

jorisvandenbossche · 2018-08-22T14:36:03Z

Quickly discussed this on hangout:

the proposed structure here is good to go, but I will investigate if it is easy to let the interface tests also be run if you run the array tests (eg importing them)
we had discussion about whether to split up the interface tests for one array-type in multiple files (similar to how the base extension tests are written), but since we didn't fully agree on that, leaving for a separate issue/PR.

I will also still some documentation / comments as discussed above (#22026 (comment))

jorisvandenbossche · 2018-08-22T14:44:09Z

I added an explanation of the scope of the tests to each of tests/extension/test_integer|interval|categorical.py. If we find that too duplicative, I can also add it only once in the __init__.py in that directory, but I personally think that is much less visible.

jorisvandenbossche · 2018-08-22T14:48:43Z

pandas/tests/extension/test_integer.py

+        super(TestArithmeticOps, self).check_opname(s, op_name,
+                                                    other, exc=None)
+
+    def _check_op(self, s, op, other, op_name, exc=NotImplementedError):


Here (below), I added quite a bit of hoops to get the tests passing. This is partly because it is not easy to have generic arithmetic tests (certainly in case of integers where we different subdtypes), and I could also simply have skipped this class completely (since we still test arithmetic operations in a more custom way in tests/arrays/test_integer.py). But since trying to get those passing, actually turned up some bugs / corner cases (see the issue links in the comments below), I think it is useful to keep this.

I think we should change this to have individual checkers (kind of like we have _check_divmod_op) which are defined in the base class, then it is straightforward to just override these here (and its not as complicated code)

That might be a good idea, but note that we would still need to do some of the below hoops for each of the operations, so not sure if it would turn out simpler.
Also, since this would touch all tests of all types and is more a refactor of the base extension tests, I would like to leave that for a separate PR. Here, I only try to split the existing integer and interval tests.

k, can you create an issue to track / refactor this.

I already opened a bunch of issues for the actual bugs (see links below). For the reworking of the tests, I don't have a concrete idea how it would look like now, so I would rather not open a vague issue (but feel free to open one if you have a concrete idea of how to refactor it)

jreback

some comments. approach is ok.

jreback · 2018-08-23T10:05:12Z

pandas/tests/arrays/test_integer.py

@@ -14,6 +13,8 @@
    Int8Dtype, Int16Dtype, Int32Dtype, Int64Dtype,
    UInt8Dtype, UInt16Dtype, UInt32Dtype, UInt64Dtype)

+from ..extension.base import BaseOpsUtil


I think we are generally using absolute imports

jreback · 2018-08-23T10:08:03Z

pandas/tests/extension/base/ops.py

@@ -69,7 +69,8 @@ def test_arith_series_with_array(self, data, all_arithmetic_operators):
        # ndarray & other series
        op_name = all_arithmetic_operators
        s = pd.Series(data)
-        self.check_opname(s, op_name, [s.iloc[0]] * len(s), exc=TypeError)
+        self.check_opname(s, op_name, pd.Series([s.iloc[0]] * len(s)),


why this change?

Similar as what Tom said below: combine doesn't work with lists (it might be we should improve combine, but that is a separate issue)

pandas/tests/extension/base/ops.py

pandas/tests/extension/test_integer.py

jreback · 2018-08-23T10:11:31Z

pandas/tests/extension/test_integer.py

+
+class BaseInteger(object):
+
+    def assert_index_equal(self, left, right, *args, **kwargs):


in general are these necessary for extensions? shouldn't we just incorporate these directly into tm.assert_frame_equal / tm.assert_series_equal ? (e.g. this was changed in tests/arrays/test_integer.py`` why not here & generally)?

IOW except for line 99 (the tm.assert_numpy_array_equal, which is related to the impl), all of this is completely generic (and just series and ops on them)

For now I simply split it, and left the logic of this file as it was before (and indeed in tests/arrays/test_integer.py I no longer used it because I needed to remove the dependency on the base classes there, and it also turned out not to be needed).
Do you remember why you added this in the first place?

I agree that in general this should not be needed (it is good to have this mechanism to overwrite it for external EAs, but you would expect assert_series_equal to work just fine for our own arrays).

Interval and categorical also don't have this

Quickly commented out this part, and all tests are still passing. So will remove and check if all passes on travis as well

jreback · 2018-08-23T10:14:18Z

pandas/tests/extension/test_integer.py

+        super(TestArithmeticOps, self).check_opname(s, op_name,
+                                                    other, exc=None)
+
+    def _check_op(self, s, op, other, op_name, exc=NotImplementedError):


I think we should change this to have individual checkers (kind of like we have _check_divmod_op) which are defined in the base class, then it is straightforward to just override these here (and its not as complicated code)

jreback · 2018-08-23T10:15:17Z

pandas/tests/extension/test_integer.py

+        super(TestArithmeticOps, self)._check_divmod_op(s, op, other, None)
+
+    @pytest.mark.skip(reason="intNA does not error on ops")
+    def test_error(self, data, all_arithmetic_operators):


was this moved? or are you just showing that this 'works'?

Yep, it's a bit a strange test. I think @TomAugspurger changed that in the sparse PR (or commented about it)

…-testing

pep8speaks · 2018-09-05T08:27:04Z

Hello @jorisvandenbossche! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/tests/arrays/test_integer.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_interval.py !
There are no PEP8 issues in the file pandas/tests/extension/base/methods.py !
There are no PEP8 issues in the file pandas/tests/extension/base/ops.py !
There are no PEP8 issues in the file pandas/tests/extension/test_categorical.py !
In the file pandas/tests/extension/test_integer.py, following are the PEP8 issues :

Line 30:13: W504 line break after binary operator
Line 31:13: W504 line break after binary operator
Line 32:13: W504 line break after binary operator
Line 33:13: W504 line break after binary operator

There are no PEP8 issues in the file pandas/tests/extension/test_interval.py !

jreback

generally looks fine. some minor comments.

jreback · 2018-09-05T11:34:20Z

pandas/tests/extension/base/methods.py

-            orig_data1._from_sequence([a + b for (a, b) in
-                                       zip(list(orig_data1),
-                                           list(orig_data2))]))
+        with np.errstate(over='ignore'):


what surfaces the error here?

When testing with Int8 dtype, you get an overflow warning for summing the scalars (the a + b below)

hmm ok, maybe we should actually move this to the integer tests rather than here then?

I think it is fine to ignore it here (it's numpy behaviour to warn, not something we need to test), but it's true that we might want to add explicit test for the behaviour we want on the Series level in the integer tests.

The behaviour currently to silently ignore it in the case of series is consistent with a Series with numpy dtype int8:

In [38]: s = pd.Series([50, 100], dtype='Int8') In [39]: s[0] + s[1] /home/joris/miniconda3/envs/dev/bin/ipython:1: RuntimeWarning: overflow encountered in byte_scalars #!/home/joris/miniconda3/envs/dev/bin/python Out[39]: -106 In [40]: s + s[::-1].reset_index(drop=True) Out[40]: 0 -106 1 -106 dtype: Int8

i understand though this may be indicative of an error in other extension types. ok for now

pandas/tests/extension/base/ops.py

jreback · 2018-09-05T11:36:06Z

pandas/tests/extension/test_integer.py

+
+@pytest.fixture
+def data(dtype):
+    return integer_array(make_data(), dtype=dtype)


@TomAugspurger should create an issue to document fixtures in the extension array tests.

I think also should have these fixtures in a conftest.py as NotImplementedError (IOW if an author doesn't implement the fixtures they should raise loudly). Once you implement they override and will work.

jreback · 2018-09-05T11:37:36Z

pandas/tests/extension/test_integer.py

+        super(TestArithmeticOps, self).check_opname(s, op_name,
+                                                    other, exc=None)
+
+    def _check_op(self, s, op, other, op_name, exc=NotImplementedError):


k, can you create an issue to track / refactor this.

jreback · 2018-09-05T11:38:29Z

rebase on master as well.

…-testing

jorisvandenbossche · 2018-09-06T10:11:22Z

Everything is passing now! Merging

…ys and /extension) (pandas-dev#22026)

jorisvandenbossche added 2 commits July 23, 2018 16:39

Split integer array tests in tests/arrays/integer and tests/extension…

f3e3085

…/integer

split interval tests

6408ee0

jorisvandenbossche added Testing pandas testing functions or related to the test suite ExtensionArray Extending pandas with custom dtypes or arrays. labels Jul 23, 2018

jorisvandenbossche added this to the 0.24.0 milestone Jul 23, 2018

jorisvandenbossche commented Jul 23, 2018

View reviewed changes

use tm.assert methods instead of class methods

de10591

jorisvandenbossche added 2 commits July 24, 2018 11:17

fix py2

1c40fd7

linter

facfc44

jreback requested changes Jul 25, 2018

View reviewed changes

get tests passing

a17deda

TomAugspurger reviewed Aug 21, 2018

View reviewed changes

pandas/tests/extension/test_integer.py Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into restructure-array…

41b2615

…-testing

add module docstring to each test file

8cf71e9

jorisvandenbossche commented Aug 22, 2018

View reviewed changes

fix rebase mistake

edcb3da

jreback requested changes Aug 23, 2018

View reviewed changes

jorisvandenbossche added 4 commits August 23, 2018 13:53

absolute import

96a9a8b

remove BaseInteger with assert overwrites

700c405

fix linter

c28cab9

Merge remote-tracking branch 'upstream/master' into restructure-array…

6ced029

…-testing

jreback approved these changes Sep 5, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into restructure-array…

c2644cb

…-testing

This comment has been minimized.

Sign in to view

pep8

db2836c

This comment has been minimized.

Sign in to view

jorisvandenbossche merged commit 4612312 into pandas-dev:master Sep 6, 2018

jorisvandenbossche deleted the restructure-array-testing branch September 6, 2018 10:11

aeltanawy pushed a commit to aeltanawy/pandas that referenced this pull request Sep 20, 2018

TST: restructure internal extension arrays tests (split between /arra…

bdca5e9

…ys and /extension) (pandas-dev#22026)

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

TST: restructure internal extension arrays tests (split between /arra…

c8d8042

…ys and /extension) (pandas-dev#22026)

		assert 'length' in result


		class TestConstructors(BaseInteger):

	def assert_index_equal(self, left, right, args, *kwargs):

	left_na = left.isna()
	right_na = right.isna()

	tm.assert_numpy_array_equal(left_na, right_na)
	return tm.assert_index_equal(left[~left_na],
	right[~right_na],
	args, *kwargs)

	def assert_series_equal(self, left, right, args, *kwargs):

	left_na = left.isna()
	right_na = right.isna()

	tm.assert_series_equal(left_na, right_na)
	return tm.assert_series_equal(left[~left_na],
	right[~right_na],
	args, *kwargs)

		self.assert_series_equal(result, expected)


		class TestArithmeticOps(BaseOpsUtil, BaseInteger):


		class BaseInteger(object):

		def assert_index_equal(self, left, right, args, *kwargs):

TST: restructure internal extension arrays tests (split between /arrays and /extension) #22026

TST: restructure internal extension arrays tests (split between /arrays and /extension) #22026

Conversation

jorisvandenbossche commented Jul 23, 2018

Choose a reason for hiding this comment

jorisvandenbossche Jul 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Jul 23, 2018

TomAugspurger commented Jul 23, 2018

codecov bot commented Jul 24, 2018 • edited Loading

Codecov Report

jorisvandenbossche commented Jul 25, 2018

jreback commented Jul 25, 2018

jorisvandenbossche commented Jul 25, 2018

jreback commented Jul 25, 2018

jreback left a comment

Choose a reason for hiding this comment

jorisvandenbossche commented Jul 25, 2018

jorisvandenbossche commented Jul 25, 2018

jreback commented Jul 25, 2018

jorisvandenbossche commented Jul 25, 2018

TomAugspurger commented Jul 25, 2018

jreback commented Jul 25, 2018

TomAugspurger commented Jul 25, 2018 via email

jorisvandenbossche commented Jul 25, 2018

jreback commented Jul 25, 2018

TomAugspurger commented Jul 25, 2018

jreback commented Jul 25, 2018

jreback commented Aug 21, 2018

jreback commented Aug 21, 2018

jorisvandenbossche commented Aug 21, 2018

jorisvandenbossche commented Aug 21, 2018

jorisvandenbossche commented Aug 21, 2018

jorisvandenbossche commented Aug 22, 2018

jorisvandenbossche commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pep8speaks commented Sep 5, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Sep 5, 2018

This comment has been minimized.

This comment has been minimized.

jorisvandenbossche commented Sep 6, 2018

jorisvandenbossche Jul 24, 2018 •

edited

Loading

codecov bot commented Jul 24, 2018 •

edited

Loading