Prevent xps.indices() from generating indices that flat index arrays
#3184
Conversation
|
Hmm, I would rather avoid filtering here, to squeeze out all the performance we can - arrays are already fairly challenging for the Hypothesis internals. I'm happy to pick up that bit over the weekend if you're satisfied by the tests though 🙂 The failures are down to #3187, not anything you're doing. |
Ok I have a solution now that just disables the one area where flat indices are generated in |
honno
force-pushed
the
xp-filter-flat-indices
branch
from
e8cfeee
to
01e2246
Compare
honno
force-pushed
the
xp-filter-flat-indices
branch
from
29c8d3b
to
6cc0862
Compare
| @@ -673,7 +685,7 @@ def do_draw(self, data): | |||
| while j < len(result) and result[j] == slice(None): | |||
| j += 1 | |||
| result[i:j] = [Ellipsis] | |||
| else: | |||
| elif self.allow_fewer_indices_than_dims: | |||
There was a problem hiding this comment.
We don't actually have a test where this is False, so it might be time to copy the pre-edit test_indices_generate_valid_indexers over to our Numpy tests.
There was a problem hiding this comment.
Did you mean True? I wrote a new test numpy/test_gen_data.py::test_basic_indices_can_generate_indices_not_covering_all_dims that checks flat indices are indeed actually generated, which parallels what array_api/test_indices.py/test_indices_generate_valid_indexers does to check they're not generated.
So I think in a logical sense I'm now covering both scenarios, but coverage is still failing. This has been a good excuse to finally start learning how to use coverage but yea haven't got round to fixing this today—let me know any quick fixes otherwise I'll explore this tomorrow.
There was a problem hiding this comment.
My mistake! I did mean False, even though there obviously are tests that execute it that way... they're just not run under coverage!
So slapping a # pragma: no branch on line 688 will fix this.
honno
force-pushed
the
xp-filter-flat-indices
branch
from
20cdd5f
to
40838d9
Compare
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
Indexing 0d shapes is now permissable
It is tested logically in both allow and disallow scenarios, but that doesn't work with coverage nicely.
honno
force-pushed
the
xp-filter-flat-indices
branch
from
ccb8c39
to
8e760ca
Compare
Resolves #3166.
I opted to simply filter the base
extra._array_helpers.BasicIndexStrategyused inside ofextra.array_api._indices(), which seems to do the trick... I didn't want to play around the internals haha. Let me know if you'd like a solution that doesn't filter itself. I'm fairly happy with the testing situation for indices now at least.