Skip to content

BUG: stray comma should be preserved for legacy printing #10120

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
Nov 29, 2017

Conversation

ahaldane
Copy link
Member

Fixes #10059

@ahaldane ahaldane added this to the 1.14.0 release milestone Nov 29, 2017
@@ -632,7 +634,7 @@ def _formatArray(a, format_function, rank, max_line_len,
if summary_insert and 2*edge_items < len(a):
leading_items = edge_items
trailing_items = edge_items
summary_insert1 = summary_insert + separator
Copy link
Member

Choose a reason for hiding this comment

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

It felt a lot cleaner to me to do this concatenation here - can lecacy be forwarded to _formatArray instead?

@ahaldane
Copy link
Member Author

Updated.

def _formatArray(a, format_function, rank, max_line_len,
next_line_prefix, separator, edge_items, summary_insert):
def _formatArray(a, format_function, rank, max_line_len, next_line_prefix,
separator, edge_items, summary_insert, legacy):
Copy link
Member

Choose a reason for hiding this comment

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

Maybe default to =False, just in case anyone is relying on these internals? Not that we have to support them, but it's easy to do so.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I want to gut this whole function to fix #9816, so let's leave this as is

@@ -487,6 +487,13 @@ def test_legacy_mode_scalars(self):
'1.1234567891234568')
assert_equal(str(np.complex128(complex(1, np.nan))), '(1+nanj)')

def test_legacy_stray_comma(self):
np.set_printoptions(legacy='1.13')
assert_equal(str(np.arange(10000)), '[ 0 1 2 ..., 9997 9998 9999]')
Copy link
Member

Choose a reason for hiding this comment

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

Needs a try-finally, maybe? Or does this have cleanup?

@eric-wieser eric-wieser merged commit 88f8310 into numpy:master Nov 29, 2017
@eric-wieser
Copy link
Member

Nevermind, all looks good. Thanks!

@lesteve
Copy link
Contributor

lesteve commented Dec 4, 2017

Thanks everyone! The scikit-learn doctests now pass on numpy master with legacy='1.13'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants