MUSE_OS icon indicating copy to clipboard operation
MUSE_OS copied to clipboard

Remove files autogenerated by Sphinx

Open alexdewar opened this issue 1 year ago • 8 comments

We don't need simulation results etc. stored in the repo. They are used for the documentation, but are generated as needed anyway. They don't appear to be used for anything else and will likely just get out of sync when the code changes.

Let's remove them and lighten the repo a bit.

alexdewar avatar May 02 '24 16:05 alexdewar

But it would seem they are not autogenerated, at least not all of them.

dalonsoa avatar May 13 '24 13:05 dalonsoa

Gah... I thought I'd got rid of the errors.

alexdewar avatar May 13 '24 14:05 alexdewar

Ok, so I think I've fixed everything now!

I've added a step to the CI to run muse --model default before checking the notebooks, which will create the requisite Results folder. I think this makes sense, because the tutorial does say to run this command at the start.

There is currently a hack in there: I'm symlinking the Results folder into docs/ because, for some reason, running-muse-example.ipynb assumes the files will be in there instead. Let me know if you'd like a less smelly solution.

I've left the Results folders under docs/tutorial-code as they actually are used by notebooks all over the place. Potentially we could remove these too at some point, but I think that's a job for another day.

alexdewar avatar May 14 '24 11:05 alexdewar

Ah, that reminds me of something I forgot to say!

So the weird thing is that the documentation builds fine, regardless of whether the results are there or not.

The reason the CI pipeline fails is because in documentation.yml it tests the Jupyter Notebooks before building the documentation, so if the notebooks fail to run, then the documentation won't get built. I might be wrong here, but it seems like two tasks are being conflated currently: check that the notebooks run and check that the documentation builds (and possibly publishing it). Really, I think the notebooks should be being checked separately, maybe as an additional step in ci.yml where all the other invocations of pytest live.

What do you think?

alexdewar avatar May 14 '24 13:05 alexdewar

Nope. This was done this way on purpose to ensure that if the notebooks didn't run, the documentation workflow failed to succeed, as a way of flagging the documentation potentially going out of sync. Otherwise, the documentation builds but, as you will see if you open it, there will be a lot of missing bits related to data not found. Or that is what should happen, at least.

dalonsoa avatar May 14 '24 13:05 dalonsoa

Ohh, I see. That makes sense.

Either way though, the user doesn't need to run anything before generating the docs with Sphinx, so I don't think we need to update the instructions.

alexdewar avatar May 14 '24 14:05 alexdewar

What do you think, @dalonsoa? Can we merge?

alexdewar avatar May 16 '24 07:05 alexdewar

OK, let's merge, but I don't understand how the documentation built locally by developers will be correct - even if the built does not fail - if Results are needed to plot stuff.

dalonsoa avatar May 16 '24 08:05 dalonsoa