-
Notifications
You must be signed in to change notification settings - Fork 327
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
Make AxesArray handle slicing correctly #451
Conversation
AxesArray needs to warn when being created with a set of axes that is incompatible with the array data. It also needs to handle slices that copy or remove an axis
Improved tests Remove axes for singleton slices using new _reverse_map attribute Restrict changes to shape
In cases where dimensions change, chances are __array_ufunc__ took care of creating an AxesArray. Return it. If there's a case where __array_function__ created an array with different dimensions than self, it will still error.
twisted axes is when axes are not adjacent but have the same label, e.g. arr = AxesArray(np.empty((1,1,1)), {"ax_spatial": [0,2], "ax_time": 1})
Also split apart basic and fancy indexing tests
Also rename _standardize_basic_indexer as _standardize_indexer
I added it before I knew what name mangling was for
TBD: should this fully return a new _AxisMapping and maybe a new indexer?
Involves processing the keys several times, with increasing standardization
Modify the standardize_indexer() function Parameterize StandardIndexer
cb8746e
to
c93ca16
Compare
dispatches to einsum, which is apparently faster anyways Still need to write tests for linalg_solve, einsum, and tensordot
c64b4e4
to
21c3b01
Compare
Also add helper methods to AxesArray class
Also gitignore env8 directory, where I keep my python3.8 environment
4d13287
to
99ceaae
Compare
Newer sphinx gives more accurate line numbers for errors
99ceaae
to
2c56053
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #451 +/- ##
==========================================
+ Coverage 93.92% 94.40% +0.48%
==========================================
Files 37 37
Lines 3602 3989 +387
==========================================
+ Hits 3383 3766 +383
- Misses 219 223 +4 ☔ View full report in Codecov by Sentry. |
Issues to add to pysindy repo, discovered during this PR but not for this PR:
|
Added a bunch of tests, mostly to check ValueErrors emitted correctly Remove default argument to _AxisMapping, AxesArray Remove __array_wrap__
Alright @znicolaou, @akaptano, this is ready for your review, if desired! After this I'd suggest we release 2.0.0. One of the main points of this PR was to removes some defaults for This PR dramatically changes the I'm going to leave this open for at least two weeks. I'd especially appreciate if you could take a look at the changes to the package outside of axes.py, which are many fewer and easier to review. Admittedly, some of these changes only make sense in debugging, when you can see that slices of the spatiotemporal grid carry the correct axis definition. |
cbff507
to
44002e0
Compare
Conflicts dealing with swapping out scipy factorial for numpy factorial Also require newaxis slice to be named.
44002e0
to
bc3fbd2
Compare
Make AxesArray handle slicing correctly
Fixes #220
It's long made sense to expose the
AxesArray
interface and document it. However, when it was created, it wasn't stable enough even for every use case inside pysindy. Indexing can remove, add, and even move around the axes in an array, which meant that properties such asax_time
andn_spatial
were unreliable when an array was indexed.FiniteDifference._accumulate()
is an example of great code that worked whichAxesArray
s, but in whichAxesArray
gave no extra understanding in debugging because the axis labels were incorrect.This PR makes basic indexing, integer advanced indexing, and boolean array indexing work. It does so by adding a a private attribute that contains both forwards and backwards maps of the axes, and modifying
__getitem__
to detect which axes are added, removed, combined, relocated, etc. The PR also expands the ufunc tests dramatically. Item 1 below is probably all that's needed to accept the PR.transpose
,roll
, andreshape
work for axesarrays.__array_finalize__
. Not sure I need all those conditionals.