poliastro / validation

Validation of poliastro against external software.
Apache License 2.0
11 stars 6 forks source link

Create ci_actions.yml #7

Closed jorgepiloto closed 3 years ago

jorgepiloto commented 3 years ago

Implements a CI GitHub action for code quality checking.

jorgepiloto commented 3 years ago

Seems to be working properly. By the way, it needs a rebase :+1:

jorgepiloto commented 3 years ago

The flake8 tool suggests to remove the setup_orekit_curdir from orekit_orbit.py. However, if this line is removed, the pytest tests/ does not execute properly... This is weird because should be fixed by the custom setup_orekit_env function in utils.py :thinking:

jorgepiloto commented 3 years ago

Cool! The orekit/tests are now being executed by the CI action. I will try to finish this tomorrow so it is finally ready for a review :rocket:

jorgepiloto commented 3 years ago

The flake8 tool suggests to remove the setup_orekit_curdir from orekit_orbit.py. However, if this line is removed, the pytest tests/ does not execute properly... This is weird because should be fixed by the custom setup_orekit_env function in utils.py thinking

It seems that orekit-data.zip holds all the org. routines. Hence, our custom function setup_orekit_env() should be defined before all the org.orekit and org.hipparchus import statements.

Hoever, this new order makes flake8 to raise the E402 error, see:

E402 module level import not at top of file

How could we proceed, @astrojuanlu? Should we ignore this error in the ci_actions.yml under the extra-flake8-options variable?

astrojuanlu commented 3 years ago

⚠️ HISTORY HAS BEEN REWRITTEN ⚠️ This will need a rebase

astrojuanlu commented 3 years ago

About your comment above @jorgepiloto , I suggest we do this instead: https://flake8.pycqa.org/en/3.1.1/user/ignoring-errors.html#in-line-ignoring-errors

jorgepiloto commented 3 years ago

Sure! I did not know about ignoring inline errors :open_mouth:

jorgepiloto commented 3 years ago

Sorry for this mess with the quality checks. I am not sure why my local configuration does not match the one from the actions YAML file :sweat:

jorgepiloto commented 3 years ago

Merging this! All tests passing now :rocket: