Avoid accessing protected class members from outside #75
Conversation
|
Thanks for the pull request @proinsias! Travis CI is failing because of #73, I will fix it this weekend and then review your contribution. |
|
Thanks @Juanlu001! This is only a minor change. This is my first code PR, so I wasn't sure whether I should submit it when the build of the master branch was failing. |
|
You've done the right thing! My policy is that |
| @@ -134,7 +138,7 @@ def _repr_latex_(self): | |||
|
|
|||
| """ | |||
| elem_names = [r"a", r"e", r"i", r"\Omega", r"\omega", r"\nu"] | |||
| elem_values = [elem._repr_latex_().strip("$") | |||
| elem_values = [elem.repr_latex().strip("$") | |||
There was a problem hiding this comment.
Not sure about this one, since it's calling _repr_latex_ of each of the elements, which is an astropy Quantity: https://github.com/astropy/astropy/blob/v1.0.7/astropy/units/quantity.py#L935
|
I added a couple of comments, unfortunately I don't think the tests cover them so I made them based on my own judgement. |
|
Sounds good! Anything else you need from me? Do you need me to undo the changes you disagreed with above? |
|
Yes please! As soon as you push the changes and the builds turn green I will merge the pull request :) |
Found a few cases of protected class members accessed from outside their modules. Fixed these by either making the member public or by using a property to access the member.
|
Okay, I reverted those specific changes, and added a comment explaining why we could keep that private method. Let me know if that looks good. |
|
Looks good to me, merging! |
Avoid accessing certain protected class members from outside
Found a few cases of protected class members accessed from outside
their modules. Fixed these by either making the member public or by
using a property to access the member.