search

chainer / chainer-chemistry

Chainer Chemistry: A Library for Deep Learning in Biology and Chemistry
MIT License
629 stars 131 forks source link

Remove IPython dependency #387

Closed dtaniwaki closed 5 years ago

dtaniwaki commented 5 years ago

I'm using Chainer Chemistry for inference API server and want to get svg data directly from the visualize method. However, visualize method call without save_filepath requires IPython which is completely unnecessary for my case. https://github.com/pfnet-research/chainer-chemistry/blob/adf740a35dca28ba9602214d070c2422b8ce3c6f/chainer_chemistry/saliency/visualizer/mol_visualizer.py#L129-L131

Could you consider removing IPython dependency from this method?

corochann commented 5 years ago

What is your expected bahavior? just return svg text without wrapping SVG method?

How about adding one kwarg use_ipython=False option in this method and when false just return svg text.

dtaniwaki commented 5 years ago

I think we should let a caller convert the output, so just returning the output of drawer.GetDrawingText() is fine, then the caller can handle it as they want.

e.g.

svg = visualizer.visualize()

# For a Jupyter user
from IPython.core.display import SVG
SVG(svg.replace('svg:', ''))

# For a user who want to save a file as png
import cairosvg
cairosvg.svg2png(bytestring=svg, write_to="foo.png")

With this interface, you can save it even as JPEG with no problem.

Moreover, we can remove unexpected None return value which is hard to debug the error detail for inference and automation cases.

corochann commented 5 years ago

https://github.com/pfnet-research/chainer-chemistry/pull/388

How about this?

corochann commented 5 years ago

Closed by #388