Rectangle patch angle attribute and patch __str__ improvements

[cdeil]

We're in the process of writing an astropy.regions package for astronomers, and we're adding an as_patch method to convert regions to matplotlib patch objects for plotting.

While writing a test for the rectangle region / patch, I noticed that matplotlib.patches.Rectangle._angle that was added in #1405 isn't exposed as a public attribute?

If that's the case, would it be possible to expose it via an angle attribute or get_angle() method?

I noticed that other parameters are exposed as a wild mix of attributes and get_ methods:

    def test_as_patch(self):
        patch = self.reg.as_patch()
        assert_allclose(patch.xy, (3, 4))
        assert_allclose(patch.get_width(), 4)
        assert_allclose(patch.get_height(), 3)
        assert_allclose(patch._angle, 5)

Presumably there are backward-compat constraints or name collisions, that make exposing patch data members in a uniform way difficult. Is there something that can be improved in general, or for Rectangle.angle specifically?

[dopplershift]

The usage of _angle inside Rectangle is simple enough (just one spot). Given that it's a constructor parameter, I don't see any reason not to just rename _angle to angle. Would you submit a PR with that?

[cdeil]

@dopplershift - I've attached a commit with the change you suggest.

While looking at the file I noticed that Rectangle.__str__ and Ellipse.__str__ don't print the angle. They should, right?
Do you want me to fix this here in this PR also?

I see that you don't have unit tests with asserts on simple data members like angle or __str__.
Do you want me to add something, or do you prefer to only maintain tests for non-trivial things, to keep it lean?

[codecov-io]

Current coverage is 61.88% (diff: 100%)

Merging #7536 into master will not change coverage

@@             master      #7536   diff @@
==========================================
  Files           173        173          
  Lines         56140      56140          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          34743      34743          
  Misses        21397      21397          
  Partials          0          0          

Powered by Codecov. Last update 45e4a46...cc8c959

[dopplershift]

Thanks! If you're willing to do the work, I'd say yes to both of your questions; I'm certainly not going to say no to more tests.

[cdeil]

OK, I'll do it here.

Side comment: when I try to run the tests following your docs, I get this:

$ python tests.py 
Python byte-compilation optimization level: 0
Traceback (most recent call last):
  File "tests.py", line 46, in <module>
    success = test(argv=sys.argv[0:1] + extra_args, switch_backend_warn=False)
TypeError: test() got an unexpected keyword argument 'argv'

I presume that's because it's picking up the MPL 1.5 from my site-packages, because I didn't install the dev version?
That's pretty error prone.
I think I saw you're changing to pytest ... +10 to that and to get rid of such a test.py file that doesn't say which MPL version was picked up or what the problem was.

[cdeil]

@dopplershift - Please review.

[dopplershift]

Looks good overall to me. Just the minor pep8 nit that the tests picked up.

[dopplershift]

Can you wrap this line so it's less than 80 columns?

[cdeil]

I've amended the last commit to wrap to 80 char here.
And I've thrown in a wedge, arc and test docstring.
Hope you like it.

[dopplershift]

Thanks for the other additions!

Unfortunately, you added a new > length 80 line.

[dopplershift]

Unfortunately, now this line is too long.

[cdeil]

I've amended my commit to fix this PEP8 and checked PEP8 locally. Should be OK now.

[dopplershift]

Looks like just a freak windows failure (one job, unrelated). Merging...

Thanks!