Closed gazzar closed 11 years ago
@gazzar, thanks for your interest in ipythonblocks! I'm not going to merge this, here's why:
Manipulating the top and bottom padding is, I think, a fairly esoteric need and the ability to do so does not support the educational goal of ipythonblocks. I agree that your change leads to simpler code in this particular case, but it's not an especially common case. I'm also not sure that your example is a better or more natural illustration than what @ctb ended up with. I think it should be possible to manipulate the BlockGrid padding via off-label CSS if it's seriously needed by individuals in the future.
The .red
, .green
, and .blue
assignments are correct as currently written. I don't assign them to the hidden variables because I want to pass the values through the property setters for validation.
Thanks again, I hope you continue to look for improvements in ipythonblocks.
Thanks for taking the time to reply with your reasons Matt. I hadn't understood the property setter chain - sorry about that. G.
No worries. :smiley:
Hi Matt, It would be nice to be able to minimize the whitespace above and below BlockGrids to avoid having to do this sort of thing: http://nbviewer.ipython.org/urls/raw.github.com/ctb/2013-pycon-awesome-big-data-algorithms/master/04-bloom-filters.ipynb Here I assume Titus Brown has made the show() method in his BloomFilter class more complicated than I think it should be, because by default there's a large amount of whitespace above and below BlockGrids, which is especially noticeable when they are 1D. This pull request achieves this change. Compare the result here, where I used my modified version of ipythonblocks: http://nbviewer.ipython.org/5180685 Even if you don't want to merge my contribution, you might like to look at the code, because I changed some private fields in your existing Block class that looked wrong to me, i.e., red, green, blue -> _red, _green, _blue. These probably aren't accessed properly in the current release code. I should have separated those into their own changeset but I was lazy.