DOC update tutorial figure & format & ENH optimize p value calculation

[PSSF23]

• Reverse color for class zero and one
• Add the missing && signs in the rst paragraphs
• Reorder examples by file names & update titles
• Update p-value function
• Optimize axis labels

[adam2392]

For the sake of code-review culture:

minor changes for figure color and code block formats

What do the changes induce? And why are they needed? Perhaps a brief description in the GH PR, or in the in-line comments.

[PSSF23]

For the sake of code-review culture:

minor changes for figure color and code block formats

What do the changes induce? And why are they needed? Perhaps a brief description in the GH PR, or in the in-line comments.

Reverse color for class zero and one, Add the missing && signs in the rst paragraphs.

[PSSF23]

No idea where the python CI error is from.

[PSSF23]

build_coleman_forest documentation is not consistent with the source. I'll update those as well as I modify the p-value tutorials.

[adam2392]

No idea where the python CI error is from.

I updated your branch wrt main. The only remaining CI errors should be from Mac-osX Python3.9/3.10 builds, which should get fixed upstream soon(?) actions/setup-python#855

If there are other CI errors, it possibly is related to your PR code changes, but we can wait and see what the CI results are.

[PSSF23]

@adam2392 My intended changes on the examples look good now. The CI error seems to relate to spin building only, which is strange as I can spin build --clean locally with no problems.

[adam2392]

Perhaps update your spin locally and see if https://github.com/neurodata/scikit-tree/blob/main/.spin/cmds.py needs to get modified at all? It's possible based on the obscure message seen in the CIs.

[adam2392]

Wanna also take a stab at quick fixing the CIs related to macos images: actions/setup-python#855 (comment)

[PSSF23]

@adam2392 overall I believe the test errors are not related to my changes. I remember you mentioned in the past that we would get rid of the meson builds anyway?

[adam2392]

We'll definitely keep the meson builds as that's the modern way of building packages with python and c++.

[adam2392]

Rn i can't fix the meson issues but it's something you can test locally by running the same command to enable coverage to see if you can replicate. Then I mentioned a few things you could look into.

For now, I don't think we can merge in changes because the failing CI are related to unit testing. We don't have the CI guarantee, so best practice would be to try to address the issue. Otw we might have to backtrack and fix it later on.

I can circle back if you don't have time after NeurIPS deadline.

[PSSF23]

@adam2392 We know the two python errors are due to unsupported python versions: actions/setup-python#855 (comment)

And the unit test errors appeared only after you merged #264, see: dace5cf

I don't mind waiting till the issue is fixed or helping you find the causes, but these errors are not related to my changes in this PR.

[adam2392]

I agree! I just meant I'd rather err on side of caution cuz the CI rn is unable to run the test suite at all. So a test could fail on one of the machines because of some obscure but meaningful reason and we have no insight.

If we merge it in, and unit tests on linux for some reason raised an issue later on when the CI build errors are fixed someone will have to come back, spend time and figure out why the unit tests failed and what LOC led to that failure.

I have done that in the past which incurs significant time on my part, so just want to see if we can hold off until someone has a Sep PR to address the CI issues. That way we are at least 100% sure the CI unit tests suggest the PR changes are fine.

I can commit to having that Sep PR but just can't do it until end of next week. In the meantime, I can point you in right direction, if you want to fix the issue beforehand. Does this sound okay with you?

[adam2392]

TLDR: your PR LGTM! Just want to wait on merging till we can rerun the CI.