holukas / dyco Goto Github PK

The paper (https://github.com/holukas/dyco/blob/main/paper/paper.md) doesn't describe alternative methods or contrast DYCO's method with them. I think giving users a good sense of the trade-offs between various alternatives is important both for communicating the value of your work and for helping users make analysis choices appropriate to their situation.

openjournals/joss-reviews/issues/2575

I'm not sure that the references in openjournals/joss-reviews#2575 follow the format JOSS wants. Please double-check this.

openjournals/joss-reviews/issues/2575

State of the Field

• Many other packages that relate to eddy-covariance data. Does DYCO perform a unique function? Is it intended to be used alongside other packages? Independently? I would love a statement mapping out how/if it related to other eddy covariance packages.

On https://github.com/holukas/dyco/wiki/Example it's unclear that I'm supposed to click the "Example Settings" link to view the code being discussed. It would be better to either embed a copy of the code in the page or at least highlight that the user should open the link for cross-referencing.

openjournals/joss-reviews/issues/2575

The link to the wiki from the README drops onto a page from which it's not obvious where the user should go. I think it would be better if the README dropped the user onto a friendly landing page in the wiki from which it would be clear where to start.

In particular the page named "DYCO Wiki" appears to just duplicate the existing landing page.

openjournals/joss-reviews/issues/2575

A few notes on the Example.

20161024100000.csv 20161024110000.csv 20161024120000.csv 20161024130000.csv 20161024140000.csv
20161024103000.csv 20161024113000.csv 20161024123000.csv 20161024133000.csv 20161024143000.csv

Please clarify that those files are available in example/input_data

Please add a CONTRIBUTING.md file or similar.

openjournals/joss-reviews/issues/2575

Statement of need

• Your About heading is pretty technical. Your Scientific Background header sets up the why DYCO is important really well. I'd switch the order of those two paragraphs, and rename the "About" heading, or provide a short summary of your scientific background in your about section.

• Who is your target audience?

Community Guidelines
DYCO could use a quick statement outlining how to:

• Report issues or problems DYCO

• Seek support if DYCOdoesn't work properly

• Because DYCO is/was tied to a ETH account, there should be a paragraph explaining how non ETH members can/should interact with DYCO when contributing, reporting issues or seeking help. Possibly this won’t be an issue if the wiki link is changed. Thanks again for migrating @holukas

Example Usage
Note, I'm currently not able to view the text of the wiki, so the below is from my notes. I can link to exact lines once the wiki link is updated

• In the text for the Results from Phase 4 figure has the sentence

"In this example, the covariance corresponds to the raw ecosystem flux of CO2, with negative values translating to CO2-uptake by the ecosystem (CO2 sink) and positive values translating to CO2-emission (CO2 source)."

This phrasing makes the axes on the Phase 4 diagrams difficult to interpret. Is this covariance in CO2 measurements? This sentence suggests that negative “covariance” values should be interpreted literally as fluxes? Clearer units on the axes could clear this up.

• The Scientific Background talks about using Dyco for N2O fluxes, but the examples are only of CO2 fluxes. Could the example show how N2O monitoring is improved?

A few README files sprinkled in the test_data folder describing the input data and its provenance would be useful.

openjournals/joss-reviews/issues/2575

I feel like it's a little unclear to me from the README what the inputs and outputs of the software are.

Is it possible to summarize very briefly what it does? I feel like there's currently a wall-of-text issue.

openjournals/joss-reviews/issues/2575

There are a number of methods and functions without documentation in, e.g., these files:
https://github.com/holukas/dyco/blob/main/dyco/analyze.py
https://github.com/holukas/dyco/blob/main/dyco/plot.py
https://github.com/holukas/dyco/blob/main/dyco/files.py

Please make sure that all public methods/functions are appropriately documented.

openjournals/joss-reviews/issues/2575

As above, to increase organization.

openjournals/joss-reviews/issues/2575

I see this

At the moment it is also necessary to include the import from pathlib import Path.
in https://github.com/holukas/dyco/wiki/Example

Why is this so? Can it be fixed?

openjournals/joss-reviews/issues/2575

I feel that reading the summary at https://github.com/holukas/dyco/blob/main/paper/paper.md does not give me a clear idea of what DYCO does. The summary mixes together background information and summary about the program.

I feel it would be clearer to state the background info in one paragraph and then explain in summary how DYCO solves the problem.

openjournals/joss-reviews/issues/2575

The description here indicates that DYCO creates a number of folders and output files.

I think this isn't really appropriate for a tool that's expected to be used within Python. If this is a library, it's outputs should be variables that can be used programmatically. If this isn't a library, then there should be a command-line way to interact with the script.

openjournals/joss-reviews/issues/2575

Please make sure there is no dead code. Either put code in a switch or remove it. Don't leave large sections commented-out. One example of the issue is here; there are others.

openjournals/joss-reviews/issues/2575

I see in test_dyco.py that at least one test is commented out:

    # def test_detect_covariance_peaks(self):
    #     """Test peak detection only"""
    #     filepath = 'test_data/test_raw_data/20161020113000.csv'
    #     segment_df = files.read_raw_data(filepath=filepath, data_timestamp_format='%Y-%m-%d %H:%M:%S.%f')
    #     lagsearch_df = lag.LagSearch.setup_lagsearch_df(win_lagsearch=[-1000, 1000],
    #                                                     shift_stepsize=10,
    #                                                     segment_name='20161031230000_iter1')
    #     lagsearch_df = \
    #         lag.LagSearch.find_max_cov_peak(segment_df=segment_df,
    #                                         lagsearch_df=lagsearch_df,
    #                                         ref_sig='w_ms-1_rot_turb',
    #                                         lagged_sig='co2_ppb_qcl_turb')
    #     lagsearch_df, props_peak_auto = lag.LagSearch.find_peak_auto(df=lagsearch_df)
    #
    #     self.assertEqual(lagsearch_df.loc[lagsearch_df['flag_peak_max_cov_abs'] == 1, 'shift'].values[0], -290)
    #     self.assertEqual(lagsearch_df.loc[lagsearch_df['flag_peak_auto'] == 1, 'shift'].values[0], -290)
    #     self.assertEqual(lagsearch_df.loc[lagsearch_df['flag_peak_max_cov_abs'] == 1, 'cov_abs'].values[0],
    #                      223.13887346667508)

Why is this? Please either fix the test or remove the dead code.

openjournals/joss-reviews/issues/2575

The "Wiki" link is now behind an ETH login.

When the account was hosted by ETH I was able to access the Wiki and the documentation and the installation instructions, so I know they are there, but now I can't. Hopefully the link just can be updated?

Once the wiki content is available again, I would consider the Installation instructions and most of Example Usage check marks complete.