Skip to content

Fix for issues #392 and #378 #429

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

Merged
merged 3 commits into from
Sep 11, 2012
Merged

Fix for issues #392 and #378 #429

merged 3 commits into from
Sep 11, 2012

Conversation

87
Copy link
Contributor

@87 87 commented Sep 7, 2012

Hi, I made an attempt to fix the current issues with insert() (#378 and #392). It should now work with all scalar types (not only integer types) and also with multidimensional inserts when the axis parameter is specified. (And added a few extra tests.)

87 added 2 commits September 7, 2012 02:00
This should fix the problems with numpy.insert(), where the input values
were not checked for all scalar types and where values did not get inserted
properly, but got duplicated by default.
@njsmith
Copy link
Member

njsmith commented Sep 7, 2012

The test failures appear to be the same old generic breakage, nothing to do with this patch.

#378 and #392 both contain test cases, can you add those to the tests in this patch so we can be sure that they are really fixed?

@87
Copy link
Contributor Author

87 commented Sep 7, 2012

Oki, I added an extra test from #378; the case from #392 is already covered in test_multidim.

@87
Copy link
Contributor Author

87 commented Sep 7, 2012

Too bad the mainline still has testing errors, otherwise the CI feedback would be a nice feature.

@njsmith
Copy link
Member

njsmith commented Sep 7, 2012

It's still more convenient to check the logs than to run the tests manually when reviewing but yeah...

Anyway, this looks good to me now, thanks :-).

njsmith added a commit that referenced this pull request Sep 11, 2012
@njsmith njsmith merged commit aed8fc5 into numpy:master Sep 11, 2012
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 participants