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
WebAgg: extracted figure_div style into css and changed layout #2825
Conversation
Set #figure-div display style to inline-block to use free space on the page more effectively. See http://www.vanseodesign.com/blog/demo/inline-block/. Additionally, set margin to 10px around every figure, so padding-bottom becomes unnecessary.
Please note that I have no idea why my patches break Travis build. It seems broken already. |
You broke pep8 compliance (added a semi-colon to a line in webagg_backend.py) |
Damn, it was hard to interleave C++, Javascript and Python yesterday. Fixed. |
@@ -14,7 +14,7 @@ | |||
$(document).ready( | |||
function() { | |||
var main_div = $('div#figures'); | |||
var figure_div = $('<div style="padding-bottom: 12px"/>') | |||
var figure_div = $('<div id="figure-div"/>') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am barely literate in css. To check my understanding, this changes from doing the styling in-line (bad) to doing the styling in the css file (good)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. But I changed the style, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, I just just picking up on the style
vs id
change.
The logging changes seem like much bigger than the style changes. @mdboom I think this one needs your attention. |
And don't worry about the 3.2 fail, that is a known problem (that I really should fix as I think I broke it). |
What's the motivation for the new logging configuration? Would a "silent" kwarg in fact meet your needs better? I'm wary of adding more rcParams... |
I need to have a way to tell WebAgg to use logging instead of print-ing. I am unaware of any other way of doing this. Always printing messages is bad for a library. I would just replace print with logging, but I guess you will find it too radical and inacceptable. |
Shutting WebAgg up is bad, either. My production app keeps track of all the events and if WebAgg reports some unexpected text, it requires attention. |
I was suggesting the "silent" kwarg would merely silence these specific messages, not everyhing, and it would be a kwarg rather than an rcParam, since if you are starting the class directly, presumably you know that you don't want these messages. Using global state (an rcParam) for that is too invasive. These messages are really just conveniences for a user starting up the server at the console, and needing instruction to open the item in their web browser. It won't ever be useful as a log message, since it doesn't really indicate unusual behavior. My question was more aimed at -- how are you calling WebAgg, and why is logging these messages helpful to you? I think you've indicated that they are not helpful, but you'd like to continue to get any unusual warnings or exceptions. |
Okay, seems that I worked out a solution which should satisfiy everybody. No rcParam-s. Old behavior is preserved. I am able to hack "WebAgg" logger to use a custom handler. The thing in logging "user messages" is to prove everything is working correct. You see, my application is asynchronous and if something blocks inside WebAgg, it goes dead. It helps to diagnose. |
Sorry, I'm still not crazy about pulling in the logging module (which we don't currently use anywhere else) just for this. Can you not replace sys.stdout with whatever you want instead, without having to edit matplotlib? |
@mdboom, we really do need to start embracing the logging module. I use it all the time for my work projects, and it is fantastic. I suggested as much back when we were talking about issues with matplotlib catching certain command-line arguments for triggering "logging" messages. I see no reason why we shouldn't start doing that now. |
I don't have a problem with thoughtfully moving to using the logging module, but it should have a MEP. There's sort of ad hoc logging in matplotlib already, because it predates the stdlib logging module -- but I don't want to do it as part of this PR, where I still don't think there's a good case for it. |
I am agree with both. We should do it, but not ad-hoc in this PR. I suspect we want to be able to set the logging handlers/levels at the module level. |
If a Tornado ioloop is already running before WebAggApplication.start() is called, it should be re-used instead of trying to start it again. This allows one to use WebAgg in his/her external Tornado application.
Okay, I got it. |
Logging module: I use it, too, but care will be required because it's global. It's a bit tricky. |
Right, that is why a library needs to treat its loggers that it sets up On Mon, Feb 24, 2014 at 2:30 PM, Eric Firing notifications@github.comwrote:
|
@mdboom Can you push the button on this one if you are happy? |
@mdboom Are you happy? :) |
WebAgg: extracted figure_div style into css and changed layout
Set #figure-div display style to inline-block to use free space on the page
more effectively. See http://www.vanseodesign.com/blog/demo/inline-block/.
Additionally, set margin to 10px around every figure, so padding-bottom
becomes unnecessary.