Confusing/Misleading docstring for cartesian_to_spherical · ManimCommunity/manim#3123

(2 评论) (0 反应) (0 负责人)Python (17,820 star) (1,378 fork)batch import

good first issue

描述

I was using cartesian_to_spherical in space_ops.py but got confused by the docstring. In there it says it returns (distance, phi, theta) however what the function calls phi and theta is not returned in that order. Because both cartesian_to_spherical and spherical_to_cartesian use this order one can convert back and forth. See https://github.com/ManimCommunity/manim/blob/main/manim/utils/space_ops.py#L736-L776 .

However I think the order is confusing, or at least inconsistent with things like the value trackers of ThreeDCamera.

I'm not sure what the way to tackle issues like this is so although I initially wanted to submit a PR fixing the docstring I don't know if that is even correct anymore.

贡献者指南

技术栈: python
领域: documentation
议题类型: documentation
难度: 2
预计时间: under 1 hour
活动状态: stale
清晰度: clear
前置要求: basic Pythonunderstanding of coordinate systems
新手友好度: 85
研究方向: The issue references the function in manim/utils/space ops.py (lines 736-776) and notes inconsistency with ThreeDCamera's value trackers in manim/camera/three d camera.py (line 87). First, read the docstring and implementation of cartesian to spherical to understand the current return order. Then, compare with the usage in three d camera.py to determine the expected order. Finally, update the docstring to accurately describe the return order, or if a deeper fix is needed, discuss in the issue before making changes.