Skip to content

Conversation

nafest
Copy link
Contributor

@nafest nafest commented Oct 3, 2017

_array2string unconditionally added a comma after the '...' that
are inserted for arrays exceeding the size threshold. Add the
separator character instead. Fixes #9777.

@nafest nafest force-pushed the remove_stray_comma branch from 6825c4d to c4cb384 Compare October 3, 2017 08:41
@@ -373,7 +373,7 @@ def wrapper(self, *args, **kwargs):
@_recursive_guard()
def _array2string(a, options, separator=' ', prefix=""):
if a.size > options['threshold']:
summary_insert = "..., "
summary_insert = "..." + separator
Copy link
Member

@eric-wieser eric-wieser Oct 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you push this inside _formatArray? It receives both arguments, so can concatenate them itself

@eric-wieser
Copy link
Member

Tests and fix looks good, just a minor refactor suggested

_array2string unconditionally added a comma after the '...' that
are inserted for arrays exceeding the size threshold. Instead, add
the separator character in _formatArray. Fixes numpy#9777.
@nafest nafest force-pushed the remove_stray_comma branch from c4cb384 to 80c624e Compare October 3, 2017 09:41
@nafest
Copy link
Contributor Author

nafest commented Oct 3, 2017

@eric-wieser: moved the concatenation of summary_insert and the separator to _formatArray

@eric-wieser eric-wieser merged commit 1f4ed32 into numpy:master Oct 3, 2017
@eric-wieser
Copy link
Member

eric-wieser commented Oct 3, 2017

Thanks for your first contibution, @nafest.

Adding a "needs release note" tag, in case we want to add a remark about this alongside all the other str/repr changes that are going into this release when we do a final pass over the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
00 - Bug 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes component: numpy._core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants