Skip to content

MAINT: Move ndarray.__str__ and ndarray.__repr__ to their own file - #9202

Merged
mhvk merged 2 commits into
numpy:masterfrom
eric-wieser:move-str-repr
Jun 1, 2017
Merged

mhvk merged 2 commits into
numpy:masterfrom
eric-wieser:move-str-repr

Conversation

@eric-wieser

@eric-wieser eric-wieser commented Jun 1, 2017

Copy link
Copy Markdown
Member

Goal here is to let the definition for __unicode__ be in the same place too (#9201).

This doesn't build, claiming there's a mismatch between the API dict and index.

What have I done wrong?

Comment thread numpy/core/src/multiarray/strfuncs.c Outdated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Seems that this isn't being picked up by the tool to build the numpy API

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You need to add things in numpy/core/setup.py -- multiarray_deps

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, just too late to help!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have done already

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope, help still needed - it still doesn't build for that reason

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

(That fixup was some other errors that I spotted that would appear once I fix this API one)

@eric-wieser eric-wieser Jun 1, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Solved - API_FILES in getapi.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the _deps listings should be unnecessary now, nothing actually reads them and for gcc compatible compilers it determines dependencies automatically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wish these things were documented a bit more clearly... Raised issue gh-9203 to remind us.

@eric-wieser eric-wieser changed the title Move str repr MAINT: Move ndarray.__str__ and ndarray.__repr__ to their own file Jun 1, 2017
@eric-wieser

Copy link
Copy Markdown
Member Author

All set, builds locally now

@mhvk mhvk added this to the 1.14.0 release milestone Jun 1, 2017
@mhvk

mhvk commented Jun 1, 2017

Copy link
Copy Markdown
Contributor

This looks all OK now. Hopefully the tests will bear this out...

@eric-wieser

Copy link
Copy Markdown
Member Author

Tests are good!

@mhvk

mhvk commented Jun 1, 2017

Copy link
Copy Markdown
Contributor

OK, even though this hasn't been out long enough for it to be necessarily noticed by all, I think this is a clear enough improvement that I'll just merge it. Then, #9201 can proceed as well.

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.

4 participants