ManimCommunity/manim

Confusing/Misleading docstring for cartesian_to_spherical

Open

#3 123 ouverte le 15 janv. 2023

Voir sur GitHub
 (2 commentaires) (0 réactions) (0 assignés)Python (1 378 forks)batch import
good first issue

Métriques du dépôt

Stars
 (17 820 stars)
Métriques de merge PR
 (Merge moyen 11j 17h) (4 PRs mergées en 30 j)

Description

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.

Guide contributeur

Direction de recherche
Le problème fait référence à la fonction dans manim/utils/space ops.py (lignes 736-776) et note une incohérence avec les value trackers de ThreeDCamera dans manim/camera/three d camera.py (ligne 87). Tout d'abord, lisez le docstring et l'implémentation de cartesian to spherical pour comprendre l'ordre de retour actuel. Ensuite, comparez avec l'utilisation dans three d camera.py pour déterminer l'ordre attendu. Enfin, mettez à jour le docstring pour décrire précisément l'ordre de retour, ou si une correction plus approfondie est nécessaire, discutez en dans le problème avant d'apporter des modifications.
Stack technique
python
Domaine
documentation
Type d'issue
Documentation
DifficultéDifficulté estimée pour un nouveau contributeur, de 1 pour un très petit changement à 5 pour un travail expert.
2
Temps estiméFourchette de temps approximative pour investiguer, implémenter, tester et préparer une pull request.
Moins d'une heure
Statut d'activitéDisponibilité apparente de l'issue : fraîche, active, ancienne, bloquée ou en attente d'un mainteneur.
Ancienne
ClartéClarté avec laquelle l'issue explique le changement attendu, les critères d'acceptation et la prochaine étape.
Claire
Prérequis
basic Pythonunderstanding of coordinate systems
Accessibilité débutantScore de 1 à 100 estimant l'accessibilité de cette issue pour un premier contributeur.
85