Fixed a bug in offsetbox #1859

Closed
wants to merge 5 commits into
from

Projects

None yet

5 participants

@jrevans

Offsetbox was using a local member before it was set. This has been corrected.

jrevans added some commits Nov 28, 2011
@jrevans jrevans Merge pull request #1 from jrevans/working
Merging from my 'working' branch to my 'master' branch?
0cfcea9
@jrevans jrevans Merge pull request #2 from jrevans/working
Merging Working back into Master
9a9bd20
@jrevans jrevans Merge branch 'master' of github.com:jrevans/matplotlib
* 'master' of github.com:jrevans/matplotlib:
00f55eb
@jrevans jrevans Merge remote-tracking branch 'upstream/master'
* upstream/master: (545 commits)
  compat.subprocess: Turn on absolute importing.
  api_changes: Document the API changes.
  Moved matplotlib.subprocess_fixed to matplotlib.compat.subprocess.
  cbook.report_memory: If ps not found, raises NotImplementedError, not OSError.
  texmanager: Ignore dvipng if it cannot be found, instead of raising OSError.
  subprocess_fixed: Gracefully handle platforms without subprocess.Popen.
  All modules now use matplotlib.subprocess_fixed instead of subprocess.
  Added new module, matplotlib.subprocess_fixed, as a replacement for subprocess.
  backend_ps: Do not write to a temporary file unless using an external distiller.
  DOC improvements on scatter plot
  MEP10 - documentation improvements on many common plots: scatter plots, imshow, matshow etc
  finance: Use context manager to close files.
  finance.fetch_historical_yahoo: Create directories leading up to cachename.
  BF - correct return type for Axes.get_title
  DOC axes now refers to the Markers module instead of concatenating docstrings
  DOC improvements on the markers module
  MEP10 - documentation improvements on the markers module
  PEP8 fixes on the markers module
  Improving triinterp_demo pylab example
  Improved test_triinterp_colinear.
  ...
a44f340
@jrevans jrevans JRE: Fixed an error where a local variable was used before it was set. ece229c
@dmcdougall
Matplotlib Developers member

+1.

Can you squash this into one atomic commit, though?

@jrevans

I am not sure what you mean by "squash this into one atomic commit". I have only made the one change.

@dmcdougall
Matplotlib Developers member

@jrevans There are extra commits loitering about. They are 0cfcea9, 9a9bd20, 00f55eb, and a44f340. You can get rid of them by doing the following (assuming you're currently on the master branch and your most recent commit is ece229c): git -i rebase HEAD~5.

An editor will pop up with instructions on how to 'fixup' your commits. You can reorder them, too.

If you're not comfortable doing this I can do it locally and merge it for you.

@efiring
Matplotlib Developers member

@jrevans, your PR includes 5 commits, but it should include only the last. The others are merge commits that are not relevant and would clutter the history. Maybe the problem arose because you did not make your change in a topic branch relative to up-to-date master. http://matplotlib.org/devel/gitwash/development_workflow.html

Is the bug you are fixing in 1.2.x? If so, the fix should go there first.

@dmcdougall
Matplotlib Developers member

@efiring I was just looking into the 1.2.x branch for this and, yes, the bug exists there too.

@dmcdougall
Matplotlib Developers member

Sorry, my mistake. It's fine in 1.2.x. That's weird. Why didn't it get merged?

You can see it here: https://github.com/matplotlib/matplotlib/blob/v1.2.x/lib/matplotlib/offsetbox.py#L1451

@dmcdougall
Matplotlib Developers member

The offending commit is: dd32575.

So, in short, there's no need to apply this to the 1.2.x branch. Sorry for the spam.

Edit: And a link for the lazy.

@pelson
Matplotlib Developers member

Presumably this is not used anywhere in the mpl codebase (and is certainly not tested).
It begs the question, can we simply delete the class? Is there anybody using this functionality? If we want to keep it, is there a test which we can add for this?

@WeatherGod
Matplotlib Developers member
@pelson
Matplotlib Developers member

I've cherry picked the last commit onto master (2920408). Thanks @jrevans.

@pelson pelson closed this Apr 18, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment