Skip to content
This repository has been archived by the owner on Jul 2, 2021. It is now read-only.

Dynamically import matplotlib.pyplot #519

Merged
merged 2 commits into from
Feb 27, 2018

Conversation

yuyu2172
Copy link
Member

@yuyu2172 yuyu2172 commented Feb 6, 2018

Related chainer/chainer#2740

matplotlib backend can now be configured after importing chainercv.

@yuyu2172 yuyu2172 added the bug label Feb 6, 2018
@yuyu2172 yuyu2172 added this to the v0.9 milestone Feb 6, 2018
@knorth55
Copy link
Contributor

knorth55 commented Feb 6, 2018

@yuyu2172 How about also importing matplotlib in the position of _check_available()?

class Hogehoge():
    def __init__():
         try:
            import matplotlib
            self.pyplot = matplotlib.pyplot
         except:
             warn('hogehoge')

@yuyu2172
Copy link
Member Author

yuyu2172 commented Feb 6, 2018

Why do we need to have a module matplotlib.pyplot as an attribute?

@knorth55
Copy link
Contributor

knorth55 commented Feb 7, 2018

i thought the problem is importing the module multiple times.
In that case, i thought its better keeping the module as attribute and reusing it

_available = True

except ImportError:
except (ImportError, TypeError):
Copy link
Member

Choose a reason for hiding this comment

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

Are there any cases that TypeError is raised?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied it from Chainer.

TypeError is sometimes called when backend is not imported properly.

related

chainer/chainer#2482

https://stackoverflow.com/questions/31328436/typeerror-constructor-returned-null-while-importing-pyplot-in-ssh

Copy link
Member

Choose a reason for hiding this comment

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

I see.

@Hakuyume
Copy link
Member

I don't think keeping a module as an attribute is good. @yuyu2172's solution looks good and enough.

Copy link
Member

@Hakuyume Hakuyume left a comment

Choose a reason for hiding this comment

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

LGTM

@Hakuyume Hakuyume merged commit c7f4287 into chainer:master Feb 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants