search

ECP-CANDLE / candle_lib

MIT License
1 stars 9 forks source link

fix tests #15

Closed rajeeja closed 1 year ago

rajeeja commented 1 year ago

pre-commit errors are due to astropy->isort->https://github.com/astropy/astropy/pull/14338

It should be fixed soon. This PR can be merged @jmohdyusof

jmohdyusof commented 1 year ago

I would like to keep develop working so we can try to figure out why docs are not building from candle_lib

rajeeja commented 1 year ago

I would like to keep develop working so we can try to figure out why docs are not building from candle_lib

develop will also fail. This has nothing to do with code, it is just some package "astropy" dependencies that are causing this. Everything runs fine locally.

rajeeja commented 1 year ago

Test things locally, this doesn't cause a failure. Not worth the time to breakdown dependencies and fix the failing package version for the sake of getting pre-commit happy.

rajeeja commented 1 year ago

This PR when approved will fix Jenkins and the failing tests.

jmohdyusof commented 1 year ago

Exactly, develop will also fail. For now it is better to have this branch fail. Once this branch builds I will merge.

Can we figure out why the docs are not building from develop?

jmohdyusof commented 1 year ago

What is the command to run pre-commit locally? We need to put it in the readme

rajeeja commented 1 year ago

What is the command to run pre-commit locally? We need to put it in the readme

pre-commit run --all-files

jmohdyusof commented 1 year ago

So right now both branches pass pre-commit locally. I guess both will fail on the server for the same reason, nothing to do with the code?

rajeeja commented 1 year ago

Exactly, develop will also fail. For now it is better to have this branch fail. Once this branch builds I will merge.

It is just the pre-commit that is failing, this is nothing to do with docs.

Docs will be a different PR.

Can we figure out why the docs are not building from develop?

Our docs are built from develop those are currently here: https://ecp-candle.github.io/Candle/candle/candle.html

In the next PR I will add a docs folder to this repo and we will have our docs locally.

jmohdyusof commented 1 year ago

Our docs are built from develop those are currently here: https://ecp-candle.github.io/Candle/candle/candle.html

Why are these docs formatted so differently?

rajeeja commented 1 year ago

Our docs are built from develop those are currently here: https://ecp-candle.github.io/Candle/candle/candle.html

Why are these docs formatted so differently?

These are not relevant to this PR, we can take that conversation to the docs repo This just fixes Jenkins failures: https://jenkins-gce.cels.anl.gov/job/CANDLE_candle_lib/69/console Without this

rajeeja commented 1 year ago

So right now both branches pass pre-commit locally. I guess both will fail on the server for the same reason, nothing to do with the code?

Right. I think you can and merge this to develop now.