ManimCommunity/manim

Confusing/Misleading docstring for cartesian_to_spherical

Open

#3123 aperta il 15 gen 2023

Vedi su GitHub
 (2 commenti) (0 reazioni) (0 assegnatari)Python (1378 fork)batch import
good first issue

Metriche repository

Star
 (17.820 star)
Metriche merge PR
 (Merge medio 11g 17h) (4 PR mergiate in 30 g)

Descrizione

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.

Guida contributor

Direzione di ricerca
Il problema fa riferimento alla funzione in manim/utils/space ops.py (righe 736-776) e nota un'incoerenza con i value tracker di ThreeDCamera in manim/camera/three d camera.py (riga 87). Per prima cosa, leggi il docstring e l'implementazione di cartesian to spherical per comprendere l'ordine di ritorno attuale. Quindi, confronta con l'utilizzo in three d camera.py per determinare l'ordine previsto. Infine, aggiorna il docstring per descrivere accuratamente l'ordine di ritorno, o se è necessaria una correzione più approfondita, discuti nel problema prima di apportare modifiche.
Tech stack
python
Dominio
documentation
Tipo issue
Documentazione
DifficoltàQuanto dovrebbe essere impegnativa per un nuovo contributor.
2
Tempo stimatoTempo stimato per completare e verificare uno scope piccolo.
Meno di 1 ora
Stato attivitàQuanto è probabile che la issue sia ancora attiva e reviewable.
Datata
ChiarezzaQuanto chiaramente la issue descrive problema, scope e risultato atteso.
Chiara
Prerequisiti
basic Pythonunderstanding of coordinate systems
Adatta ai principiantiQuanto la issue è adatta a contributor alla prima esperienza.
85