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

Development status #94

Closed
pnsaevik opened this issue Oct 30, 2023 · 5 comments
Closed

Development status #94

pnsaevik opened this issue Oct 30, 2023 · 5 comments

Comments

@pnsaevik
Copy link

In my opinion, a JOSS submission should be marked as Development Status :: 5 - Production/Stable on PyPI, and it's version number should not start with 0 (see also, e.g., https://martin-thoma.com/software-development-stages/).

This package is already quite mature, so there is no reason it should not be marked as "Stable", even with the issues I previously raised on GitHub. With one exception: It should be clear to the user which part of the API is considered stable, and which is open to change. At the very least, examples in the tutorials should have "frozen syntax" and should not be changed except at infrequent major version upgrades.

Before bumping up the release to version 1.0.0, I suggest you do the following:

  1. Fix minor issues already raised
  2. Clearly state which part of the API is fixed, either summarized on the API Reference front page or individually for each exposed function.
  3. If you are unhappy with any part of the public API, now is the time to fix it. Do an internal review among developers and internal users:
    3.1. Are the variable/class/function/parameter names intuitive?
    3.2. Is the naming scheme consistent?
    3.3. Are we happy with the module/class organization? Should something be moved?
    3.4. Remember that parameter names, their ordering, and possible default values are part of the public API. Go through each function and make changes if necessary, before declaring the function as a part of the public "frozen" API.

This issue is a part of a JOSS review at openjournals/joss-reviews#5967

@MaceKuailv
Copy link
Owner

Thank you! I was not aware of the convention of releases. This is very helpful. I think I will bump it up to 1.0.0 when both of you are happy with it.

I don't think there is going to be a lot of changes to the syntax or keyword arguments on functions and classes used in the tutorial examples. Before submitting this paper, we spend a lot of time discussing pretty much exactly the third item and already made a lot of changes. For example, PR #58.

I am not entirely sure what you mean by labeling the part of API that is fixed. Do you have an example on how other people did that? Thanks!

@pnsaevik
Copy link
Author

Great, looks like you've already spent significant amount of work to polish the API. The most common way of separating "public" and "internal" API would be to only include the public API in the official documentation. Internal methods/classes/modules are excluded from the build script which creates the documentation. Exclusion is done either by prefixing the methods with an underscore, or by keeping an explicit list of objects to include/exclude. Unit testing is focused primarily on the public API, since the internal API should be open to change and developers should be encouraged to refactor frequently.

Some "unfinished" API objects can be included in the documentation if it's clearly separated from the public part. For instance, xarray har a section called "xarray internals" where some near-mature API objects are documented (https://docs.xarray.dev/en/stable/internals/index.html).

Another method is to include a warning when presenting internal API. Like in matplotlib._api: https://matplotlib.org/stable/api/_api_api.html

My impression is that https://macekuailv.github.io/seaduck/network_of_object.html should be defined as the public API, while the rest of https://macekuailv.github.io/seaduck/api_reference.html should be regarded as internal API / implementation details. Am I right? If so, I think a clearer separation would be beneficial.

@MaceKuailv
Copy link
Owner

OK, I think that's very doable and useful. I will make it clear in the table of content. I will also look into the public files and identify the non-public functions and underscore them.

@MaceKuailv
Copy link
Owner

The final changes I made in PR #95 implemented that. You can take a look after the website finishes building.

@pnsaevik
Copy link
Author

Great, the change to the documentation resolves the issue. There is still some potensial for polishing the documentation, but this is "further work".

If you want a small sample ROMS dataset for extending your package, take a look at https://github.com/pnsaevik/ladim_plugins/blob/master/ladim_plugins/chemicals/forcing.nc. It's an exercept from a fjord model in southern Norway.

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

2 participants