AnBowell/whittaker-eilers

a few observations

Closed this issue · 4 comments

This is excellent!

I have a few observations:

The Python docstring for the main class, for some reason, is not displayed in the contextual help in VSCode in a Jupyter Notebook. It does show in the popup help. This must be some subtle formatting issue, not sure.

The lmbda parameter name should be re-done. It's not worth saving one keystroke there. Just call it lambda.

It might be perhaps helpful to set default values for lambda and order. I'm not sure here, but it seems like it could benefit the ease of utilization.

Thank you for the excellent package.

To add:

The docstring for the main class does show up in the contextual help if I download the .pyi file locally, rename it to .py and import the class directly from it.

It would be nice if you could link your Towards Data Science article in the main README of the repo, as a convenient howto.

Thanks for appreciating the package and taking the time to give some feedback!

I've managed to reproduce the issues you were seeing with the docstrings, so I'll see if I can change the formatting there - it's very fiddly. I'll include the article in the README and I'll also embed the README in the Python module docstring if I can.

As for the lmbda parameter, it's not so much about saving keystrokes but rather that lambda is a reserved keyword in Python for creating lambda functions, so I'll leave that as is.

I'll also look into whether I can get default values passed through as I do agree it would make it much easier to use!

I've now changed where the documentation is in v0.1.2. I've tested it in a clean environment, installing from pip, and using Juypter in VSCode I get both doc strings and type information now.

In the end, I decided against adding defaults as lmbda can really differ depending on the data you use. If I were to choose a standard default it may smooth the data imperceptibly, or just result in a straight line. I did add a suggested range in the docs though, hopefully encouraging everyone to experiment with the value.

The docstring looks good, and the suggested ranges are useful. Thank you!