ManimCommunity/manim

Confusing/Misleading docstring for cartesian_to_spherical

Open

#3.123 aberto em 15 de jan. de 2023

Ver no GitHub
 (2 comments) (0 reactions) (0 assignees)Python (1.378 forks)batch import
good first issue

Métricas do repositório

Stars
 (17.820 stars)
Métricas de merge de PR
 (Mesclagem média 11d 17h) (4 fundiu PRs em 30d)

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.

Guia do colaborador

Direção de pesquisa
A issue referencia a função em manim/utils/space ops.py (linhas 736-776) e observa uma inconsistência com os rastreadores de valor de ThreeDCamera em manim/camera/three d camera.py (linha 87). Primeiro, leia a docstring e a implementação de cartesian to spherical para entender a ordem de retorno atual. Em seguida, compare com o uso em three d camera.py para determinar a ordem esperada. Por fim, atualize a docstring para descrever com precisão a ordem de retorno ou, se for necessária uma correção mais profunda, discuta na issue antes de fazer alterações.
Pilha de tecnologia
python
Domain
documentation
Tipo Issue
Documentação
DifficultyDificuldade de implementação estimada para um novo contribuidor, de 1 para alterações muito pequenas a 5 para trabalho de nível especializado.
2
Tempo estimadoUm intervalo de tempo aproximado para um colaborador experiente investigar, implementar, testar e preparar um pull request.
Menos de 1 hora
Status da atividadeQuão disponível o issue aparece agora: novo, ativo, obsoleto, bloqueado ou aguardando entrada do mantenedor.
Antigo
ClarityCom que clareza o issue explica a mudança esperada, os critérios de aceitação e a próxima etapa.
Claro
Prerequisites
basic Pythonunderstanding of coordinate systems
Simpatia para novatosUma pontuação de 1 a 100 que estima o quão acessível este issue é para colaboradores iniciantes.
85