Skip to content
This repository has been archived by the owner on Oct 14, 2023. It is now read-only.

Consider using fixtures in tests #447

Closed
astrojuanlu opened this issue Sep 7, 2018 · 3 comments
Closed

Consider using fixtures in tests #447

astrojuanlu opened this issue Sep 7, 2018 · 3 comments

Comments

@astrojuanlu
Copy link
Member

There are a lot of repeated code in the tests, mainly sample data for orbits. I have been experimenting with pytest fixtures lately and I think they are a great idea. They can even be parametrized, which might help repeating the same test with different kinds of orbits.

https://docs.pytest.org/en/3.8.0/fixture.html

astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Oct 28, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Nov 3, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Nov 9, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
@shreyasbapat
Copy link
Member

Working on #451 I remembered that I may have left this issue half solved. I will search for the tests where I can use fixtures. Please let me know if you have any modules in mind.

@astrojuanlu
Copy link
Member Author

While reviewing this, I realized that perhaps we could take https://github.com/poliastro/poliastro/blob/master/src/poliastro/tests/tests_twobody/test_angles.py and replace for loops with pytest.mark.parametrize. And also remove this leftover print:

print(M, ecc, M.value, ecc.value)

@astrojuanlu
Copy link
Member Author

I don't see many places to replace current tests with fixtures. I imagine we could think about cases to improve or expand the tests by testing more kinds of orbits, but I am not sure whether pytest.mark.parametrize would be more suitable to the task.

In fact, I am going to close this issue, because it's "a solution in search of a problem" and might push us in the wrong direction.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants