Skip to content
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

Issue #341: "RunTime error when printing astropy.table.Table row" #344

Merged
merged 2 commits into from Aug 17, 2012

Conversation

jwoillez
Copy link
Member

First commit is a test case that fails.
Second commit is a workaround.
Fundamental issue is at: numpy/numpy#385

@eteq
Copy link
Member

eteq commented Aug 17, 2012

Thanks for looking into this, @jwoillez - just as an FYI, you can attach PRs directly to issues you create instead of issuing a separate PR. You can either use a script I made for this (https://gist.github.com/1750715) or if you have the hub command line tool (http://defunkt.io/hub/) installed, it's pull-request command will do this.

(It's also fine to issue a separate PR - just wanted to mention that you can do this also)

@jwoillez
Copy link
Member Author

It indeed felt awkward open in a separate PR. Thanks @eteq for the interesting pointers. On the learning curve...

@astrofrog
Copy link
Member

This seems fine to me, but I'll let @taldcroft make the final call

@taldcroft
Copy link
Member

@jwoillez - thanks for working on this and sorry for the slow response. I'll look at this today. Everything looks good, I just want to first actually reproduce the issue and take a quick look to satsify myself that the numpy bug should not be a problem anywhere else. Also, I'll run this through my stage branch to do the full regression test on all platforms before merging (if you or @astrofrog haven't done so).

@taldcroft
Copy link
Member

Looks good and passes on jenkins for debian and OSX. I'm merging now.

taldcroft added a commit that referenced this pull request Aug 17, 2012
Issue #341: "RunTime error when printing astropy.table.Table row"
@taldcroft taldcroft merged commit cddf7cf into astropy:master Aug 17, 2012
@taldcroft
Copy link
Member

Now we're playing whack-a-mole. It looks like the windows repr of an int array is (1L, 4L) instead of (1, 4) as in linux and mac. We could update the test so it just checks that no exception is raised. Replace

assert format(row, "") == "<Row 0 of table\n values=(1, 4)\n dtype=[('a', '<i8'), ('b', '<i8')]>"

with

format(row, "")

or maybe

assert format(row, "").startswith("<Row 0 of table")

These latter two versions should still exercise the original bug.

@astrofrog
Copy link
Member

I like the last one, because it only depends on astropy.table rather than Numpy (i.e. if Numpy change their string representation of an array, we have to update the text).

Do you want me to fix it? I'm happy to, but just want to make sure we don't both do it.

@taldcroft
Copy link
Member

Yes, please fix this. Thanks.

@astrofrog
Copy link
Member

Should be fixed in b7da66c

@astrofrog
Copy link
Member

Just confirming the tests now pass on Windows

keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
Issue astropy#341: "RunTime error when printing astropy.table.Table row"
@jwoillez jwoillez deleted the issue_341 branch November 18, 2013 09:33
astrofrog pushed a commit to astrofrog/astropy that referenced this pull request Jun 12, 2019
Mark sphinx extensions as parallel-safe
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.

None yet

4 participants