Skip to content
This repository was archived by the owner on Dec 21, 2023. It is now read-only.

Conversation

@dhivyaaxim
Copy link
Contributor

This PR resolves #2236
Fixed Jupyter notebook inline title

@TobyRoseman TobyRoseman requested a review from znation September 13, 2019 20:05
Copy link
Contributor

@znation znation left a comment

Choose a reason for hiding this comment

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

One comment needs to be addressed; also, the Travis CI run is failing (not sure why).

</style> \
</head> \
<body> \
<h1> '+ title +' </h1> \
Copy link
Contributor

Choose a reason for hiding this comment

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

title needs to be HTML encoded; otherwise some values of title could cause malformed HTML. You can use the HTML.escape in Python 3 and cgi.escape in Python 2 for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @znation ! Will check and raise PR

@dhivyaaxim dhivyaaxim requested a review from znation September 16, 2019 12:41
Copy link
Contributor

@znation znation left a comment

Choose a reason for hiding this comment

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

Just one comment, otherwise this is ready to go in. Thanks!

from PIL import Image

import base64
import cgi
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put these imports within the if/else conditions where they are used. Otherwise one of the imports may fail on Python 2 or Python 3 (I think the html module is new in Python 3).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@znation Thanks for your suggestion! Changes are implemented and request for PR

@dhivyaaxim dhivyaaxim requested a review from znation September 18, 2019 16:35
@znation znation merged commit ce615ff into apple:master Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Jupyter notebook SFrame.explore ignores title parameter

2 participants