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

Fixed custom jinja2 templates being ignored when setting template_path #4035

Merged
merged 4 commits into from Aug 16, 2013

Conversation

mlhenderson
Copy link

IPython/html/notebookapp.py was ignoring user specified templates by setting the jinja2 environment to the default template directory before updating template_path. This fix allows a user defined template_path to propagate to the jinja2 environment.


# create a jinja2 environment using the template path and store it in settings
settings["jinja2_env"]=Environment(loader=FileSystemLoader(settings["template_path"]))

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 that be above settings_override ?

Copy link
Author

Choose a reason for hiding this comment

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

The problem is that having the Environment initialized before
settings_overrides is applied means that any custom template_path setting
is ignored. The template_path used in the original code will always be the
default, regardless of what you set template_path to. This keeps
everything in the settings dict, but allows for the user to override
template_path when the jinja2 environment is created.

On Thu, Aug 15, 2013 at 4:08 AM, Matthias Bussonnier <
notifications@github.com> wrote:

In IPython/html/notebookapp.py:

     )

     # allow custom overrides for the tornado web app.
     settings.update(settings_overrides)
  •    # create a jinja2 environment using the template path and store it in settings
    
  •    settings["jinja2_env"]=Environment(loader=FileSystemLoader(settings["template_path"]))
    

Shouldn't that be above settings_override ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/4035/files#r5787706
.

Copy link
Member

Choose a reason for hiding this comment

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

ah yes,...but then settings["jinja2_env"] cannot be overridden.
I'm too tired to find the right logic, but is should be possible wit a condition to deal with each cases ... will sleep on it.

Copy link
Member

Choose a reason for hiding this comment

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

All you should need to change is the initial assignment of template_path:

template_path = settings_overrides.get('template_path', os.path.join(os.path.dirname(__file__), "templates"))

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, that's a good approach. I've updated accordingly.

On Thu, Aug 15, 2013 at 3:46 PM, Min RK notifications@github.com wrote:

In IPython/html/notebookapp.py:

     )

     # allow custom overrides for the tornado web app.
     settings.update(settings_overrides)
  •    # create a jinja2 environment using the template path and store it in settings
    
  •    settings["jinja2_env"]=Environment(loader=FileSystemLoader(settings["template_path"]))
    

All you should need to change is the initial assignment of template_path:

template_path = settings_overrides.get('template_path', os.path.join(os.path.dirname(file), "templates"))


Reply to this email directly or view it on GitHubhttps://github.com//pull/4035/files#r5803867
.

@@ -173,7 +173,7 @@ def init_settings(self, ipython_app, kernel_manager, notebook_manager,
mathjax_url=ipython_app.mathjax_url,
config=ipython_app.config,
use_less=ipython_app.use_less,
jinja2_env=Environment(loader=FileSystemLoader(template_path)),
jinja2_env=Environment(loader=FileSystemLoader(template_path))
Copy link
Member

Choose a reason for hiding this comment

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

minor nitpick, but can you avoid removing this comma?

Copy link
Author

Choose a reason for hiding this comment

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

Done, but in my defense technically I was making the IPython Notebook more
efficient...

On Thu, Aug 15, 2013 at 4:42 PM, Min RK notifications@github.com wrote:

In IPython/html/notebookapp.py:

@@ -173,7 +173,7 @@ def init_settings(self, ipython_app, kernel_manager, notebook_manager,
mathjax_url=ipython_app.mathjax_url,
config=ipython_app.config,
use_less=ipython_app.use_less,

  •        jinja2_env=Environment(loader=FileSystemLoader(template_path)),
    
  •        jinja2_env=Environment(loader=FileSystemLoader(template_path))
    

minor nitpick, but can you avoid removing this comma?


Reply to this email directly or view it on GitHubhttps://github.com//pull/4035/files#r5804840
.

@minrk
Copy link
Member

minrk commented Aug 16, 2013

thanks, merging.

minrk added a commit that referenced this pull request Aug 16, 2013
Fixed custom jinja2 templates being ignored when setting template_path
@minrk minrk merged commit f231e2c into ipython:master Aug 16, 2013
minrk added a commit that referenced this pull request Aug 16, 2013
…etting template_path

IPython/html/notebookapp.py was ignoring user specified templates by setting the jinja2 environment to the default template directory before updating template_path.  This fix allows a user defined template_path to propagate to the jinja2 environment.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Fixed custom jinja2 templates being ignored when setting template_path
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

3 participants