Skip to content

Rebase ipython_directive on top of recent ipython updated version #5925

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
7 commits merged into from Jan 18, 2014
Merged

Rebase ipython_directive on top of recent ipython updated version #5925

7 commits merged into from Jan 18, 2014

Conversation

ghost
Copy link

@ghost ghost commented Jan 13, 2014

ipython/ipython#4570

We'd prefer to get rid of the duplication, but we have some tweaks in place
which ipython does not.

I rebased everythingon top of ipython to generae a clean diff, if they pick it up
we can make the transition, otherwise we synched for now.

cc @takluyver, if you're interested, check the diff between 2e3c608 and 82b8c3a
for an unobstructed view.

@jorisvandenbossche, can you check this out and let me know if I missed anything?
I included the cython magic fixes you pointed out.

related #5221

@jorisvandenbossche
Copy link
Member

Some early feedback (not yet tested thoroughly):

@ghost
Copy link
Author

ghost commented Jan 13, 2014

Thanks, I'll look at those tomorrow.

@ghost
Copy link
Author

ghost commented Jan 14, 2014

@cpcloud , the cython fix you put in ipython_directive should find it's way upstream,
do you remember what the issue was?

@ghost
Copy link
Author

ghost commented Jan 14, 2014

Ok, that was very strange (and a nightmare to track down). a snippet I add long ago to faq.rst which demonstrates... monkey patching (q @jreback gloating) borked the numbering by ipython directive.

Took another crack at GH5142, after rereading the issues.

@jorisvandenbossche, seal of approval?

@jreback
Copy link
Contributor

jreback commented Jan 14, 2014

@y-p monkey patching pandas is ok :) its just other packages BY pandas which is a problem :)

@ghost
Copy link
Author

ghost commented Jan 14, 2014

Yep, except when that break stuff as well. case in point.

@jorisvandenbossche
Copy link
Member

@y-p I can't test it today anymore, but I will do tomorrow.

@ghost
Copy link
Author

ghost commented Jan 14, 2014

Thanks, no rush. You've just proven to have a better eye for it.

@jorisvandenbossche
Copy link
Member

A big +1 on this!

I did build the docs, and it seems good:

  • all prompt / code blocks seem OK (correct alignment, counting, etc)
    • there is a small difference with ipython master, with your version the Out [] prompts are red, with ipython's version they are blue :-) doesn't matter, but don't know why.
    • In the current version of the docs, we didn't have Out [] prompts. Now there are, if we want there is an option to change that, but for me this is OK (it's a little more crowdier, but it's what users also see in there console).
  • regarding the io.rst / unicode issue:
  • okexcept is working fine, errors in code are sent to console
  • no warnings anymore! thanks to the moving of the template (well, there were 7 warnings, but all related to references to the deleted api.rst)

So as long you don't forget to put back the api docs before merging, this is looking good.

@ghost
Copy link
Author

ghost commented Jan 15, 2014

Twice, I told myself twice to remember to get rid of that commit before pushing. sigh

The coloring and Out prompts are not intentional but no harm I guess, would rather move on.
The question marks are due to using decode(foo, errors='replace') which always succeds and returns
unicode.
I'm still unclear about the unicode issue. The current docs contain a string which is not valid utf8
and I fully expected that to create all kinds of trouble, not sure why it doesn't.

I'll give the unicode bit another look, I was sure the issue was solvable. Am still holding out
for some of this to get merged upstream into ipython.

Thanks.

@ghost ghost mentioned this pull request Jan 15, 2014
@jorisvandenbossche
Copy link
Member

In every case, the question marks in the output is better than not being able to build it (as long as the docs on the website are build on linux where it is not a problem).

And indeed, the prompts/coloring is certainly not an issue to me, just remarked the difference.

@jorisvandenbossche
Copy link
Member

I don't know if it is useful, but it seems a little bit related: in matplotlib/numpydoc they had also an issue with special characters: numpy/numpydoc#1, matplotlib/matplotlib#2529

@ghost ghost mentioned this pull request Jan 18, 2014
ghost pushed a commit that referenced this pull request Jan 18, 2014
Rebase ipython_directive on top of recent ipython updated version
@ghost ghost merged commit 8c71949 into pandas-dev:master Jan 18, 2014
@ghost ghost deleted the PR_rebase_ipython_directive branch January 18, 2014 16:31
@ghost ghost restored the PR_rebase_ipython_directive branch January 18, 2014 16:35
@ghost ghost deleted the PR_rebase_ipython_directive branch January 18, 2014 16:35
@ghost ghost restored the PR_rebase_ipython_directive branch January 18, 2014 16:35
This pull request was closed.
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.

2 participants