cosmiq/cometts

Comet Time Series Toolset for working with a time-series of remote sensing imagery and user defined polygons

cometts's Issues

Make GitHub release and link version to Zenodo and paper

(This issue is part of a review of the CometTS submission to JOSS. I will add a comment in that thread with a list of all items from my review when I've finished.)

The Zenodo archive should correspond to a tagged release. Currently the latest un-tagged source code is archived on Zenodo.

At the end of the review process, I'd suggest making a release on GitHub and updating the Zenodo archive and paper to reflect this.

Improve notebooks

The notebooks run smoothly, thank you for providing them. Here are some small changes that could make them much more user-friendly.

Each notebook should have a title and one sentence explaining what it does. Ideally some inline Markdown cells should tell the user what to change and what's going on.

Instead of a single script pasted in the first cell, I suggest you separate the cells logically so the user can step through the analysis. This might be: one cell for imports, one cell for user-defined variables not specified in a widget, one cell per widget definition, and a final "run" cell. Or, if you don't want to use cells, please add header comments as you did in CSV_Creator.ipynb.

See examples of clean notebooks and example scripts here and here.

Finally, please populate the widget fields in CometTS_Visualizer.ipynb with relative paths to your San Juan example. This will allow the user to immediately see what CometTS can do.

• Add headers and explanatory text to notebooks
• Separate cells or add header comments for clarity
• Populate default example in CometTS_Visualizer.ipynb

(JOSS review thread)

Correct paths to example data

When running the ARIMA_Plotting notebook, the user will have to edit the input_CSV variable to point to the relative path to your San Juan example. Otherwise the notebook will fail with an error.

I suggest that this should be specified as a relative path (CometTS/VIIRS_Sample/San_Juan_ARIMA_Output.csv), or alternatively include step-by-step instructions in each example notebook.

• Change any absolute paths to relative paths in example notebooks

(JOSS review thread)

LSTM for time series ?

Condense example in README and link to separate documentation

Your README is a first entry point to the project, and should summarize what it does, how to contribute and get help, and where to find more details. Currently I feel that the detailed workflow makes it difficult to get a overall picture of the package.

• Migrate README walkthrough to notebooks, or condense example
• Provide easily-digestible workflow steps

Any examples in the README should be pretty short; it's better to give more detail in separate documentation that you link to. See the verde package's README and documentation for a good example (published in JOSS!).

I suggest that you move most of the Workflow example section to a separate notebook or notebooks, and provide only a short walkthrough of the key steps. Something like this, with the links pointing to the more detailed documentation:

CometTS workflow

To use CometTS, you must define your area of interest, provide a CSV file documenting how your imagery is organized, and then run one of the CometTS analysis tools. This usually takes the following steps

1. Outline and download your AOI with a service like geojson.io
2. Organize your imagery and document it with the CometTS.CSV_It tool
3. Analyze your data using:
   • CometTS for trend analysis (optionally, mask unwanted clouds and other features with --maskit option)
   • or CometTS.ARIMA for averaging and anomaly detection
4. Plot the results using one of the plotting notebooks

(JOSS review thread)

Edit paper for style and clarity

The paper has been improved, but I feel like some detail is missing. The main issues I see are:

• The core contribution could be better defined in the second paragraph.

Right now, the paper states that CometTS provides new functionality in moving window statistics, extracting a time series from an ROI, and cloud masking. SITS, for one, has ARIMA functionality, and TIMESAT and SITS provide cloud/QA filtering, so I don't think these are the new, key features of CometTS. You should probably emphasize other unique advantages of CometTS as a rapid inspection workflow while mentioning these as important features.

• There is some redundant use of the first person. For instance, you could edit the first sentence to remove "We have developed .. that we called" to more directly describe the software. This would be something like: "Comet Time Series (CometTS) is a set of Python notebooks that enables analysis and visualization of a time series of satellite imagery for the data science and geographic communities."

• I think "geography community" is a better term for the set of researchers in this field.

• Typo in the last paragraph of the paper ("calucluated" -> "calculated").

(JOSS review thread)

Remove or clarify commented-out code

It's nice to see the tests pass, but there are a few instances of commented-out code in your tests and the example notebooks. You should remove these if they aren't necessary, or clarify if they can be run in certain cases (like you do in ARIMA.py and elsewhere).

• Remove old code and print statements in ARIMA.py: L45-46, L120, L155-160, L204-205, 210, 235, 237
• Remove old code from CometTS.py. There are many in the plot functions like L139, L291-296, L357 and 375, L476-481, as well as old testing calls like the NDVI plot at L847.
• Remove old code from tests. It looks like you rewrote the code to use pd.testing instead of standard assertions, but many commented-out assertions remain (e.g. line 62-65 and 67 of Comet_test.py). There's also a block-commented code snippet at the end of file that should be deleted.

This will make the examples more accessible and make it easier to extend or contribute to CometTS without wading through scratch code.

(You also might consider removing the statement "No plotting testing included, do your own damn plotting if you have a problem." I get that it's unrealistic to test all possible uses of bokeh and matplotlib in the CometTS workflow, but this could be a little off-putting to someone who might want to contribute.)

(JOSS review thread)

Add instructions for running tests

I suggest that brief instructions to install pytest and run it on CometTS/tests be added to either the README or CONTRIBUTING documents. It's not in requirements.txt so won't necessarily be in the development environment.

This will help contributors verify their changes don't break your codebase and help users determine what works if they install from a development branch.

• Add testing instructions

(JOSS review thread)

Other Issues From Review

• Version: Does the release version given match the GitHub release (v1.0)?
• Documentation - A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is? Add audience to statement of need (https://joss.readthedocs.io/en/latest/review_criteria.html#a-statement-of-need)
• Paper - A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is? Add audience to statement of need (https://joss.readthedocs.io/en/latest/review_criteria.html#a-statement-of-need)
• I think it would be better to intro something like ‘user defined polygon identifying a region of interest (ROI)’ and use ROI for the remainder of the paper.
• Figures: please provide descriptive captions to the three figures.
• I feel like the paper title could be smooth like this: 'Comet Time Series Visualizer: CometTS'
• that we call -> called
• (CometTS) -> ([CometTS])
• a times series of satellite imagery -> time series satellite imagery.
• -> The tool aims to enable research into population estimation, land use detection, or natural disaster monitoring using a variety of data types.
• series of satellite imagery -> series of satellite images
• variation), -> variation)
• visualizes -> provides a visualization of
• from user drawn polygons. -> from a user defined region of interest.
• define GIS as geographic information systems
• can only be used to analyze individual -> are limited to analysis of individual
• CometTS then produces -> CometTS produces
• within the polygon -> region of interest
• of most relevance -> most relevant
• standard statistics -> standard statistical measures / standard test statistics
• Many of the details in the final paragraph could be summarized in a shorter sentence like “CometTS output includes user-specified statistics such as mean, median, and quartiles, and the package offers masking functionality to remove clouds and snow from the area of interest.” Then the remaining space could outline key research use cases in more detail.
• Expand introduction, including appropriate citations to related work
• Edit manuscript to remove workflow details in favor of suggested research use cases
• Remove ® symbol from “Python ®”
• Version: The Zenodo archive and paper should correspond to a tagged release on Github (#6)
• Acknowledge in the documentation whether CometTS is
  a. a workflow for processing/visualizing raster time series, or
  b. tools for workflows; i.e. it's unclear how many of the functions are reliant on outputs from other pieces.
• Acknowledge the key features that distinguish CometTS from other processing/visualization tools for raster time series, e.g. TIMESAT.
• Test and indicate in the README which imagery sources and formats are compatible with CometTS.

Add doc strings for major functions; reformat function names per PEP 8

Every significant function should include a simple docstring to describe what it does, its arguments, and any outputs.

Related to this, I suggest renaming your functions according to conventional Python style. For example, Do_Zonal_Stats would become do_zonal_stats. Using more descriptive names, like calculate_zonal_stats, would be even better. This should be easy to do across the package and notebooks by find and replace or sed.

As noted in @zhampel's review, it would be good to run a format checker like flake8 on your codebase which would catch issues like this. It looks like you did this in your response, but there are still some small whitespace issues.

• Add docstrings/function headers
• Reformat function names consistently

(JOSS review thread)