Skip to content

edits to dev docs#3428

Open
petrelharp wants to merge 1 commit intotskit-dev:mainfrom
petrelharp:devdocs
Open

edits to dev docs#3428
petrelharp wants to merge 1 commit intotskit-dev:mainfrom
petrelharp:devdocs

Conversation

@petrelharp
Copy link
Contributor

I"ve gone through the dev docs and done a lot of the things in them. They are great, thanks @jeromekelleher. I have only minor edits.

However: maybe they need something else, since the docs don't build for me locally? I get

$ make clean
$ make
/home/peter/projects/tskit-dev/tskit/docs/<breathe>:: WARNING: undefined label: 'tables_8h_1aa0a88b2ce7cdc013a86b6a52b63920a5'
/home/peter/projects/tskit-dev/tskit/docs/<breathe>:: WARNING: undefined label: 'tables_8h_1aa0a88b2ce7cdc013a86b6a52b63920a5'
/home/peter/projects/tskit-dev/tskit/docs/c-api.rst:579: WARNING: undefined label: 'tables_8h_1aa0a88b2ce7cdc013a86b6a52b63920a5'

To test out changes to the *code*, you can change to the `python/` subdirectory,
and run `make` to compile the C code.
If you then execute `python` from this subdirectory (and only this one!),
If you then execute python commands from this subdirectory (and only this one!),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can't run python; you can only run uv run python etc

To install the hook:

```bash
$ uv run prek install
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was already said above

@codecov
Copy link

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.92%. Comparing base (1835ea3) to head (9cd5b07).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3428   +/-   ##
=======================================
  Coverage   91.92%   91.92%           
=======================================
  Files          37       37           
  Lines       32153    32153           
  Branches     5143     5143           
=======================================
  Hits        29556    29556           
  Misses       2264     2264           
  Partials      333      333           
Flag Coverage Δ
C 82.70% <ø> (ø)
c-python 77.34% <ø> (ø)
python-tests 96.40% <ø> (ø)
python-tests-no-jit 33.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Python API 98.69% <ø> (ø)
Python C interface 91.23% <ø> (ø)
C library 88.86% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Note that this guide covers the most complex case of adding a new function to both
the C and Python APIs.

0. Draft a docstring for your function, that describes exactly what the function
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since I'm not building locally I haven't checked if we can actually start a list at zero. but I think this should be first, not last?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, sgtm

@jeromekelleher
Copy link
Member

Hmm - what version of doxygen are you using? I've got

$ doxygen --version
1.9.8

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, but we should get to the bottom of the doxygen issue (which is annoying!).

Note that this guide covers the most complex case of adding a new function to both
the C and Python APIs.

0. Draft a docstring for your function, that describes exactly what the function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, sgtm

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