Hi @bruno-pannunzio : are you using a Mac? Do you have any idea why the _blend_darken and _blend_lighten Cython functions emit RuntimeWarning: invalid value encountered? Did you notice this on your system before? On Ubuntu and Windows no warnings are raised as expected. Those functions are checking for NaN input, just like the other Cython ufuncs do. We can probably mark the failing tests as xfail on macOS. What do you think?

https://github.com/phasorpy/phasorpy/actions/runs/10841795383/job/30086395612?pr=115#step:5:51

Since _blend_darken and _blend_lighten are not used in PhasorPy, I disabled the failing tests on macOS. Probably XCode erroneously optimizes the isnan checks away.

The decreased test coverage is due to an increase in lines of code for typing, which is not tested.

Hi @cgohlke. I have no warnings running _blend_darken and _blend_lighten in my Mac. Also tests are not failing. I am not aware what this problem might lie.

I think we can skip those tests for Mac as you already did.

Hi @bruno-pannunzio : thanks for checking. I suspect it is a compiler optimization issue. GitHub Actions might have an older XCode version installed and it looks like this is fixed in the latest version.

This seems like a good time to revisit the Repo-Review from the Scientific Python Library Development Guide.

• They want ruff and mypy enabled in pre-commit. We discussed and postponed both.

• They suggest enabling enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"] for mypy. I'll look into that (fixed in Configure mypy enable_error_code #116).

• Instead of blacken-docs we use blackdoc.

• I do not agree on using either ReadTheDocs or a task runner.

• Not sure why they recommend setting a custom pre-commit CI message. The default "[pre-commit.ci] pre-commit autoupdate" looks better than the replacement they suggest (fixed in Configure mypy enable_error_code #116).

• The "upload-artifact" detection looks too strict on their side. We should update or replace the outdated release-pypi.yml before the release though.

@bruno-pannunzio : Any thoughts?

Hi @cgohlke, sorry I couldn't answer before.

• Instead of blacken-docs we use blackdoc.

I think blackdoc is better since it can also be used to apply black formatting to docstring in python files, which I find it quite useful. On the other hand blacken-docs only supports Markdown, reStructuredText, and LaTex files, if I understood correctly. So I think we should stick to blackdoc.

• I do not agree on using either ReadTheDocs or a task runner.

I don't think we need tox or another task runner in our case, as you said, at least for now we are doing finde without it. Maybe we can revisit later if the community asks for it? Regarding ReadTheDocs, I think people are using it more and more to access documentation, but I wouldn't mind not having it since we have a very detailed and working documentation already. I believe you don't want to use ReadTheDocs to avoid duplicating documentation or is there another reason?

• The "upload-artifact" detection looks too strict on their side. We should update or replace the outdated release-pypi.yml before the release though.

I am not aware of the benefits of using the suggested format for using unique names for "upload-artifact".

you don't want to use ReadTheDocs to avoid duplicating documentation or is there another reason?

They insert Ads.

I am not aware of the benefits of using the suggested format for using unique names for "upload-artifact"

I think they are referring to upload-artifact@v4, which requires unique names. Anyway, we already use unique upload names in the wheel builder (release-pypi.yml is outdated):

Excellent, I totally agree. I then believe that we are covered regarding the Scientific Python Library and have deliberately made the modifications we think are better, at least for now. But maybe it's a good idea to revisit this with some regularity to make sure everything and we are aligned.

maybe it's a good idea to revisit this with some regularity

Good idea. I opened issue #118 as a reminder.