Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add documentation (e.g. docstrings) #273

Open
dkarrasch opened this issue Nov 22, 2018 · 6 comments
Open

Add documentation (e.g. docstrings) #273

dkarrasch opened this issue Nov 22, 2018 · 6 comments

Comments

@dkarrasch
Copy link
Contributor

I realized that currently there is no documentation of any interpolation options. There is only the README on the github page, which refers to the PDF containing some of the derivations, but this seems to be incomplete in terms of documenting options, and pointing out some specifics of the options if applicable. So we could expand on some options that require additional information (like Periodic() BCs) in the README, or start writing docstrings. Any opinions?

This came up in #272, and it may after all affect only that Periodic() case, I don't have an overview if it's worth it writing docstrings for other options.

@tomasaschan
Copy link
Contributor

Hm. I know I wrote lots of docstrings at one point - but it looks like most of them were dropped in #224... 🤔

Re-adding them (well, the relevant parts) would of course be a welcome contribution, but I don't blame you if it feels overwhelming 😄

@timholy
Copy link
Member

timholy commented Nov 22, 2018

Despite heroic efforts in the past, documentation is still a weak point of the package. I think the docstrings deleted in #226 were no longer relevant (e.g., they described the generated-function based API), and on balance I think it added quite a few missing ones: as just one example, before #226 interpolate itself didn't have a docstring. But more would be better.

@timholy
Copy link
Member

timholy commented Nov 22, 2018

While on topic of the boundary-condition options, another thing that I think is largely missing (?) is tests that show the boundary conditions actually do what we claim they do. I thought about adding those in #226, but it was such a big change that at a certain point it became all I could do just to get that over the finish line and then clean up the (many) messes it caused.

@tomasaschan
Copy link
Contributor

@timholy Just to make sure you don't read anything into the above message that I didn't intend: it was absolutely not my intention to assign blame anywhere by pointing out where it seems like many of the docstrings that did exist were removed; rather, it was an attempt at pointing out a resource where one might find useful information for creating docstrings where they are missing. (And it didn't help that I managed to point to the wrong place - I of course intended to point at #226 rather than #224...).

There is something at https://interpolationsjl.readthedocs.io/en/latest/ too, but it seems that it's pretty outdated; getting whatever docs we have to show up there (or in a similar place) would probably be a good start on spotting weak points and starting to fill them in.

@Crown421
Copy link

I would like to follow up here, and mention that this package could use a "Getting started" section.
The General Usage section did not give me enough information to apply the package, and the "Interpolation Algorithms" section starts with some code containing an a and an A that I can't see defined.
From what I understand so far, the array being given is assumed to be the function outputs on a uniformly spaced [0,1] grid, but I can't find an explicit mention of this.

It also possible that I am wrong, and just haven't found the right passage, but I just wanted to give some feedback as someone who just wanted to start interpolating.
Maybe once I have gotten it to work and understand why, I can submit a PR for the documentation, but until then I wanted to leave it here.

@timholy
Copy link
Member

timholy commented Jan 21, 2020

It would be a huge contribution to give this package some serious documentation.

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

No branches or pull requests

4 participants