PyFixate / Fixate

Framework for hardware test fixtures and automatic test environments
MIT License
22 stars 16 forks source link

Docs: add API reference based on docstrings #194

Closed josh-marshall-amp closed 2 months ago

josh-marshall-amp commented 2 months ago

I saw this internal doc pop up, and went ... hey, isn't that what good API docs are for? And given this is on Github, pretty sure we can automate it, and then it'll be kept up to date, included in the release page and if docstring-based will also be accessible in IDEs.

So this PR:

And in order to make it actually useful, either as part of this or as a followup, should:

Sample

DMM_package

DMM_helper

References

josh-marshall-amp commented 2 months ago

@jcollins1983 I deliberately overrode pre-commit for my commits, since there is a stack of stuff to fix overall and I wanted to get the concept up for feedback first... no point spending that time in case it was going counter to other folks' views on the direction for fixate. So I've marked them resolved without fixing first... but would do so before merge.

josh-marshall-amp commented 2 months ago

@jcollins1983 I also like to keep 'content' vs 'process' PRs separate... so my intention with the changes to the docstrings themselves were just as an example, and I can imagine those could be updated by those who know Fixate or are using it more than I. We should discuss whether Google- or numpy-style docstrings are a preference, as they seem to be online (much more readable and "pythonic")

I'm in favour of improving our documentation. I'd also be in favour of automating the build of the documentation as part of the PR process.

This, but do these as separate PRs. 👍

josh-marshall-amp commented 2 months ago

There's still quite a number of warnings on docstrings, so these will need to be cleaned up at some stage.

Still need the discussion on docstring style... but it does look like a mix of rst :param name: and the google style is acceptable to Sphinx, can migrate slowly.

Haven't added the docs built to the github workflow, but that's probably worthwhile even if we're not using them just yet. I don't want to fail builds on doc warnings yet though.

clint-lawrence commented 2 months ago

I've enabled PR preview builds on read the docs. Hopefully that works on the next push, but there might be other config needed. I'm not 100% sure.

https://docs.readthedocs.io/en/stable/pull-requests.html

josh-marshall-amp commented 2 months ago

I've pinned the docs build to 3.12 as requested. Confess I don't really quite get basepython vs the evnlist matrix... and how all that interacts with the build given we're still using the 3.8 build?

jcollins1983 commented 2 months ago

Still need the discussion on docstring style... but it does look like a mix of rst :param name: and the google style is acceptable to Sphinx, can migrate slowly.

I don't really have a strong preference, but I think I lean towards Google style, it looks a bit easier to read IMO.

josh-marshall-amp commented 2 months ago

I added a docs readme. Besides that, anything else for this PR?

I suggest we merge this, and then the next discussion can be about the structure and style of the docs themselves.

There was the discussion re: style for the examples but that can wait too.

clint-lawrence commented 2 months ago

I added a docs readme. Besides that, anything else for this PR?

I suggest we merge this, and then the next discussion can be about the structure and style of the docs themselves.

There was the discussion re: style for the examples but that can wait too.

Yes to all of that from me.

The only unresolved comments are from @jcollins1983 about if we should keep or remove TEST_SEQUENCE. I think that is a discussion for elsewhere.

jcollins1983 commented 2 months ago

I added a docs readme. Besides that, anything else for this PR? I suggest we merge this, and then the next discussion can be about the structure and style of the docs themselves. There was the discussion re: style for the examples but that can wait too.

Yes to all of that from me.

The only unresolved comments are from @jcollins1983 about if we should keep or remove TEST_SEQUENCE. I think that is a discussion for elsewhere.

Agree that the conversation RE TEST_SEQUENCE can happen elsewhere/later.