cosmiq / cometts Goto Github PK
View Code? Open in Web Editor NEWComet Time Series Toolset for working with a time-series of remote sensing imagery and user defined polygons
License: Apache License 2.0
Comet Time Series Toolset for working with a time-series of remote sensing imagery and user defined polygons
License: Apache License 2.0
(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.
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.
CometTS_Visualizer.ipynb
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.
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.
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:
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
--maskit
option)The paper has been improved, but I feel like some detail is missing. The main issues I see are:
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").
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).
ARIMA.py
: L45-46, L120, L155-160, L204-205, 210, 235, 237CometTS.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.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.)
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.
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.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.