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

Accents in notebook names and in command-line (nbconvert) #4732

Merged
merged 7 commits into from Jan 25, 2014
Merged

Accents in notebook names and in command-line (nbconvert) #4732

merged 7 commits into from Jan 25, 2014

Conversation

marcmolla
Copy link
Contributor

The Problem

Current stable version had some issues when you try to use accents in the notebook names or in the command line. I had problems with:

  • Use accents in the notebook name
  • Use command-line arguments with accents. In my case I tried to use SphinxTransformer.author = my_name for setting the author in latex documents and my_name included accents.
  • Convert to pdf a notebook with accents in its name.

The two first issues are solved in the development version but the last one is still there.

The proposed solution

I added new test cases in the nbconvert test folder for testing those issues. These tests will be useful for maintaining the accents support.

I fixed the issue with the pdf postprocessor with the accents in file names. There were two problems: A mix between str and unicode characters and also an encoding problem in windows (arguments are received in cp437 from the dos console but we must use cp1252 for calling pdflatex subprocess). I saw some code that wrap subprocess.Popen calls in the IPython.utils so I guess that this fix could be implement in a better way...

filename = filename.encode('cp1252')
else:
filename = filename.encode('utf-8')
#END_HACK
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't the command be a unicode object instead of the bytestring?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, it should be unicode but there is a problem (bug?) when using windows and python 2.x:
http://changilkim.wordpress.com/2013/03/10/some-python-subprocess-popen-tips/
http://bugs.python.org/issue19264

I tested the subprocess.Popen in both linux and windows and the problem is only with the combination of win + python 2.X (in python 3.X is fixed).

I'll update the code for using unicode (requires a minor fix) and I'll put the hack only for win+py 2.x.

"""
with self.create_temp_cwd(['nb*.ipynb']):
self.call('nbconvert --to latex nb1_* '
'--SphinxTransform.author="análisis"')
Copy link
Member

Choose a reason for hiding this comment

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

SphinxTransform doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's SphinxTransformer.author...I'll update this test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I just see that in master branch the latex template is not using sphinx. So this test is not valid.

times = 'time' if count == 1 else 'times'
self.log.info("Running %s %i %s: %s", command_list[0], count, times, command)
with open(os.devnull, 'rb') as null:
stdout = subprocess.PIPE if not self.verbose else None
for index in range(count):
p = subprocess.Popen(command, stdout=stdout, stdin=null)
p = subprocess.Popen(command, stdout=stdout, stdin=null, shell=True)
Copy link
Member

Choose a reason for hiding this comment

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

This probably shouldn't be shell=True since command is a list.

Copy link
Member

Choose a reason for hiding this comment

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

If you remove the shell=True, then this should be ready for merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't notice your previous comment. I'm going to update the code right now

@damianavila
Copy link
Member

Yep, I think is ready 👍 (btw, having the "tilde" stuff working in nbconvert is nice for my spanish named notebooks 😉, thanks!)

minrk added a commit that referenced this pull request Jan 25, 2014
Accents in notebook names and in command-line (nbconvert)
@minrk minrk merged commit 3e415e7 into ipython:master Jan 25, 2014
@takluyver
Copy link
Member

@minrk, this seems to be causing test failures on your Mac build bots. Can you investigate?

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.3/lib/python3.3/unittest/case.py", line 384, in _executeTestPart
    function()
  File "/Users/minrk/env/sp33/lib/python3.3/site-packages/IPython/nbconvert/tests/test_nbconvertapp.py", line 193, in test_accents_in_filename
    self.call('nbconvert --log-level 0 --to python nb1_*')
  File "/Users/minrk/env/sp33/lib/python3.3/site-packages/IPython/nbconvert/tests/base.py", line 154, in call
    raise OSError(stderr)
OSError: [NbConvertApp] WARNING | pattern 'nb1_*' matched no files

@minrk
Copy link
Member

minrk commented Jan 25, 2014

I'll look into it.

@minrk
Copy link
Member

minrk commented Jan 25, 2014

@takluyver #4873 makes the test pass on my machine.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Accents in notebook names and in command-line (nbconvert)
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