Skip to content

BUG: fix stray comma in _array2string #9815

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 1 commit into from
Oct 3, 2017

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