search

WSWUP / agweather-qaqc

Visualized QA/QC of weather station data
https://wswup.github.io/agweather-qaqc/
Apache License 2.0
19 stars 11 forks source link

Usability Review #22

Closed dostuffthatmatters closed 5 months ago

dostuffthatmatters commented 6 months ago

This issue is related to the review process for JOSS: https://github.com/openjournals/joss-reviews/issues/6368

Installation

It would be great to support an installation without conda because it add a quite heavy dependency that is not necessary. You could simply use Pip (using a requirements.txt file). I personally recommend PDM because it produces a pyproject.toml file that complies with PEP standards - hence even though you can use PDM as a developer, users (or a CI environment) can install the project using Pip. Poetry and Pipenv are alternatives to PDM, but the establish their own metadata format - hence users/CI environments would have to also install the respective package.

I also recommend putting environment.yml/requirements.txt/pyproject.toml files in the project's root because users and environment management tools do not have to search for the package metadata.

The .gitignore file does not include Python specific files. Hence, when I create a virtual environment inside the project, Git will show that there are 2000+ new files. You can use https://www.toptal.com/developers/gitignore to generate a good .gitignore file (maybe exclude files from python and all major operating systems).

FYI, the tests also pass with Python 3.12. Since Feature releases 3.X are backwards compatible (with a few very minor exceptions), you could consider supporting any Python version >=3.9,<4.0.

Functionality

Save the XLSX output files in more formats (CSV, Parquet, etc.), so that it easier to look at them when not having Microsoft Excel installed.

Do not only save HTML files but also static images (PNG/SVG). Makes it easier to look at the results and embed them in slides/documents. Also, since the HTML files fetch the Bokeh JS library, they only work offline if that JS library is present in cache.

Other than that, the plots are nicely done!

Packaging

Just an idea for future releases.

You might consider distributing it as an installable package via PyPI. This way, users could install it using pip install agweather-qaqc and running it inside another project instead of having to manually set up a dedicated directory for it.

If you choose to use PDM or Poetry, both make it quite easy to publish a package to PyPI. Be careful with version tagging then, because you cannot overwrite a version once it is published.

cwdunkerly commented 6 months ago

When you have a moment, please pull the latest and check the new .gitignore against your venv files.

I used the toptal tool for Windows, Python, MacOS, and Linux, as well as any specific files I knew would be created by running the code.

cwdunkerly commented 5 months ago

I have .csv's as an option for output files, and I have created an issue for future development over implementing static images as outputs.

The latter will require more work as the HTML version is used as part of identifying intervals of data that need to be corrected.

dostuffthatmatters commented 5 months ago

@cwdunkerly Awesome - good progress!