Skip to content

Fix for #146#147

Merged
ChrisRackauckas merged 2 commits into
SciML:masterfrom
sharanry:sy/fix_indexing
Jun 28, 2021
Merged

Fix for #146#147
ChrisRackauckas merged 2 commits into
SciML:masterfrom
sharanry:sy/fix_indexing

Conversation

@sharanry

Copy link
Copy Markdown
Contributor

@sharanry

sharanry commented Jun 28, 2021

Copy link
Copy Markdown
Contributor Author

This is a temporary fix that shouldn't affect anything else. More thorough testing of existing code will probably be required to prevent this from happening in the future.

@ChrisRackauckas

Copy link
Copy Markdown
Member

I was thinking it might just be missing an Int... dispatch? Something like https://github.com/SciML/RecursiveArrayTools.jl/pull/147/files#diff-a6e6da66354c0871e3f4bae3523909853bd38bdc881c3826ab93091c96eb8fd8R57 but then arbitrary layers deep.

@sharanry

sharanry commented Jun 28, 2021

Copy link
Copy Markdown
Contributor Author

I think the problem with such a dispatch is that it doesn't cover colons?

Also, I am not sure which code you are referring to. The given link is for this PR's diff.

@ChrisRackauckas

Copy link
Copy Markdown
Member

@sharanry

sharanry commented Jun 28, 2021

Copy link
Copy Markdown
Contributor Author

@ChrisRackauckas
I am planning to add a bunch of tests based on this report. The coverage has been deteriorating with time.
Could we merge this (if everything is okay) and make these additional tests a separate PR?

@ChrisRackauckas

Copy link
Copy Markdown
Member

Yeah, let's merge and see. Today isn't going to be a clean day.

@ChrisRackauckas ChrisRackauckas merged commit bdbca6b into SciML:master Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2.13-> 2.14: broken API or breaking change ?

2 participants