Skip to content

Fixing doctest failing in construct_table.rst - #2431

Merged
astrofrog merged 1 commit into
astropy:masterfrom
bsipocz:fixing_failing_automodapi_test
May 6, 2014
Merged

astrofrog merged 1 commit into
astropy:masterfrom
bsipocz:fixing_failing_automodapi_test

Conversation

@bsipocz

@bsipocz bsipocz commented May 4, 2014

Copy link
Copy Markdown
Member

This addresses the doctest failing in astropy.table mentioned in #2429

@astrofrog

Copy link
Copy Markdown
Member

I wonder whether this will work on all platforms - for instance Windows or 32-bit platforms? What is the usual way to deal with this @embray @mdboom?

@bsipocz

bsipocz commented May 4, 2014

Copy link
Copy Markdown
Member Author

Well, I didn't think about it :( Still have to learn to think in the bigger picture.
I've tested it only locally with python2.7 and 3.3, but should have checked on our obsolete server (64bit) with python 2.6.4, which gives the original format.

Sorry for the noise, now I doubt it's the right approach.

@astrofrog

Copy link
Copy Markdown
Member

@bsipocz - it's a corner case, so not obvious - and in fact, it may not actually be an issue. I just seem to remember things like this causing issues in very specific instances in the past.

@mdboom

mdboom commented May 5, 2014

Copy link
Copy Markdown
Contributor

We could just use an ellipsis instead, i.e.:

array([1, 4]...)

@embray

embray commented May 5, 2014

Copy link
Copy Markdown
Member

Sorry I should have mentioned sooner (I was doing this on Friday though and ran out the door before I had a chance) but I actually already fixed this in a branch where I was fixing Windows test failures.

There's a simple workaround for this particular case to just use print instead--then Numpy won't add the dtype to the output either way.

@embray

embray commented May 5, 2014

Copy link
Copy Markdown
Member

Meanwhile, I'm working on a separate PR (won't be ready for 0.4 though) that adds support for environment markers to the .. doctest-skip directive (for example one can write .. doctest-skip:: platform == 'win32')

@eteq

eteq commented May 5, 2014

Copy link
Copy Markdown
Member

So just so we're clear, @embray, you have a branch/soon to be issued PR that addresses this, or should @bsipocz update this now?

@embray

embray commented May 5, 2014

Copy link
Copy Markdown
Member

Either way really. If this is updated and merged first then it doesn't matter.

@embray

embray commented May 5, 2014

Copy link
Copy Markdown
Member

Though if so I would just rebase this so that there's a just a single commit using the workaround I suggested, if at all.

@bsipocz

bsipocz commented May 6, 2014

Copy link
Copy Markdown
Member Author

Done.

@astrofrog

Copy link
Copy Markdown
Member

Thanks @bsipocz!

astrofrog added a commit that referenced this pull request May 6, 2014
Fixing doctest failing in construct_table.rst
@astrofrog
astrofrog merged commit f6ac80f into astropy:master May 6, 2014
@embray embray modified the milestones: v0.3.2, v0.4.0 May 6, 2014
@embray

embray commented May 6, 2014

Copy link
Copy Markdown
Member

This affects v0.3.x too so I changed the milestone.

astrofrog added a commit that referenced this pull request May 8, 2014
Fixing doctest failing in construct_table.rst
Conflicts:

	docs/table/construct_table.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release Docs table testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants