Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cookiecutting - build docs api #51

Merged
merged 15 commits into from
Jul 27, 2024
Merged

Conversation

8bitsam
Copy link
Contributor

@8bitsam 8bitsam commented Jul 26, 2024

Build the doc API and ensure the makefile properly builds docs. Also removed a duplicate epydoc/ folder from docs/ (there is already one in docs/source/).

This should be one of the last, if not the last, PRs before the cookiecutter workflow is finished.

@sbillinge
Copy link
Contributor

@8bitsam Thanks for this.

I don't want to merge this without the docs.yml github workflow in here so we can test that the docs build ok in ci (I guess you have checked that they build correctly locally). Please can you add that to this PR.

But then we need the rest of the workflows merged on a different PR.

There seem to be a bunch of things that you are missing that makes me a bit nervous about the rest of the work. I feel that I am finding things you are not finding, but you are not finding things that I am not finding. This is ok in general (I have spent more time looking at these things than you), but it would give me more confidence that you are really digging into the code and the process and understanding what we are trying to do here if from time to time you were finding things that I have missed and raising questions about them......I would feel more supported! ;)

@8bitsam
Copy link
Contributor Author

8bitsam commented Jul 27, 2024

The docs.yml workflow was added in PR #42, it's just not running since it's set only to do so when commits are pushed to main. There's a couple options here:

  1. Make it run on a push to any branch.
  2. Make it run only on a push to main or cookie (this staging branch for cookiecutter edits).
    Either way, we can always revert it when we make a PR to merge the cookie branch into main.

Other than that, I think I've gotten everything in the package to reflect the cookiecutter/diffpy.utils. There are some differences, like how we have conda-recipe/ and rever.xsh (and this is also a C++ package). Another difference is that we're not using pytest here, but instead unittest, though the infrastructure to migrate is in place.

Out of these differences we would need to decide what we want to keep/remove and for those things we want to remove, we could do so when merging with main (would likely only be a few commits max).

Pertaining to this PR, right now there's still the doc/source/epydoc and doc/examples directories. We can decide what exactly we want to do with that/if we want to format examples into the other parts of the doc (added by cookiecutter).

@sbillinge
Copy link
Contributor

The docs.yml workflow was added in PR #42, it's just not running since it's set only to do so when commits are pushed to main. There's a couple options here:

1. Make it run on a push to any branch.

2. Make it run only on a push to main or cookie (this staging branch for cookiecutter edits).
   Either way, we can always revert it when we make a PR to merge the cookie branch into main.

Other than that, I think I've gotten everything in the package to reflect the cookiecutter/diffpy.utils. There are some differences, like how we have conda-recipe/ and rever.xsh (and this is also a C++ package). Another difference is that we're not using pytest here, but instead unittest, though the infrastructure to migrate is in place.

Out of these differences we would need to decide what we want to keep/remove and for those things we want to remove, we could do so when merging with main (would likely only be a few commits max).

Pertaining to this PR, right now there's still the doc/source/epydoc and doc/examples directories. We can decide what exactly we want to do with that/if we want to format examples into the other parts of the doc (added by cookiecutter).

Thanks for this detailed summary. It is very helpful.

  1. sorry, I am not sure why I didn't see the docs.yml workflow, I see it now. I am juggling too many packages! Let's do 2., so change it so it runs on push to cookie and we will revert. We can use this for testing.
  2. Since this is c++ we may want to run tests on all supported platforms and python versions.
  3. We may want to have a convo with Andrew about where to keep the conda-forge build files. Either we want to (a) keep them all with the code as Pavol did, or (b) have them built by cookiecutter (the current approach) or (c) keep them in a separate conda-forge repo. We actually have one on billingegroup or diffpy (I don't remember) but I don't think it is very up to date. I think in general we want to use the cookiecutter approach, which means removing conda-forge, but double-checking that the cookiecutter release workflow generates them correctly and everything builds correctly. I would need you to test that.
  4. we can remove rever.xsh, we are not using it any more.
  5. Can you make a proposal regarding epydoc and and the doc/examples. Please work with Tieqiong who is dealing with similar things for diffpy.structure. Please look at the current docs for pdffit2 and the new docs generted by our cookie api workflow and the docs here. Pavol used epydoc and I think we are using a different approach to get the api docs to build, but look at both sets of documentation and see if there is a straightforward way of getting them to the same level of professionalism, but that can build and deploy without manual intervention, in the same way for all packages. That is the goal basically, if that makes sense.

@sbillinge
Copy link
Contributor

@8bitsam I took a quick look and I couldn't find separate docs for pdffit2. They seem to be part of PDFgui documentation. at diffpypdfgui.readthedocs.

@8bitsam
Copy link
Contributor Author

8bitsam commented Jul 27, 2024

@8bitsam I took a quick look and I couldn't find separate docs for pdffit2. They seem to be part of PDFgui documentation. at diffpypdfgui.readthedocs.

There are docstrings in the python code, though not formatted the way the sphinx documentation recommends. I just pushed a bunch of commits that should build the docs better, there seems to be a git permissions issue though that is causing problems with the workflow/

@sbillinge sbillinge merged commit e42b9f6 into diffpy:cookie Jul 27, 2024
3 checks passed
@sbillinge
Copy link
Contributor

@8bitsam I'm trying the merge to see if it will build on push. If not, we may want to try and push it into main and see if the issue is that the deploy is from a build of main or sthg like that and we can't test it on cookie

@sbillinge
Copy link
Contributor

In that case, let's finish the cookie work (items above) and then we can merge into main.

@8bitsam
Copy link
Contributor Author

8bitsam commented Jul 27, 2024

Ok, I'll make a PR for a couple of those, since removing rever might be too small for a single PR.

@sbillinge
Copy link
Contributor

@8bitsam the documentation build ran ok on push to cookie and gh-pages branch seems to be properly populated. I expect the docs will appear at diffpy/github.io/diffpy.pdffit2 shortly

@8bitsam
Copy link
Contributor Author

8bitsam commented Jul 27, 2024

Great, now we just need to decide where examples/ goes. I think epydoc/ can be removed, since sphinx is just replacing its functionality, but I'll discuss with Tieqiong. In regards to examples, we could do something like diffpy.utils where we change it into a notebook-style example instead of a python file so it's more readable.

@8bitsam 8bitsam deleted the cookie-api-build branch July 27, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants