[PyOpenSci] Reviewer #1 Comments

[Zeitsperre]

Generic Issue

• xclim version: 0.40.0
• Python version: Any
• Operating System: All

Description

There are some issues with our documentation that should be addressed. (see: pyOpenSci/software-submission#73 (comment))

The following comments received from first PyOpenSci reviewer, @aguspesce (Thank you!)

---

Review Comments

Thank you for the opportunity to review this code. The package looks very good and a lot of effort has been put into its development. I only have minor suggestions and observations, so I will enumerate them in order of priority.

About the documentation

I found some issues on the documentation website that don't look well.

• The type annotations is a great tool, but sometimes it might confuse users when they read the API documentation.
  For example, when a function has a lot of parameters, the type annotation can clutter its documentation.
  So I suggest using the sphinx_autodoc_typehints extension in docs/conf.py which hide the type annotations when sphinx creates the documentation. (Contributing and overall doc comments #1308)

• The documentation for indicators and indices has a format problem when it describes the return part.
  For example, xclim.indicators.atmos.cffwis_indices has multiple descriptions and this makes the return part difficult to understand:
  Returns dc (DataArray) – Drought Code (drought_code), with additional attributes:
  description: Numeric rating of the average moisture content of deep,
  compact organic layers.dmc : DataArray Duff Moisture Code (duff_moisture_code),
  with additional attributes: description: Numeric rating of the average
  moisture content of loosely compacted organic layers of moderate depth.ffmc :
  DataArray Fine Fuel Moisture Code (fine_fuel_moisture_code), with additional
  attributes: description: Numeric rating of the average moisture content
  of litter and other cured fine fuels.isi : DataArray Initial Spread Index
  (initial_spread_index), with additional attributes: description: Numeric
  rating of the expected rate of fire spread.bui : DataArray Buildup Index
  (buildup_index), with additional attributes: description: Numeric rating
  of the total amount of fuel available for combustion.fwi : DataArray Fire
  Weather Index (fire_weather_index), with additional attributes: description:
  Numeric rating of fire intensity. (Deprecate mf_flag call signature in xclim.ensembles, expose calendar conversion utilities, fix _gen_returns_section bug #1317)

• The API documentation is a very long page, and it is difficult to navigate and find definitions. This can be improved by adding a table of contents. (Contributing and overall doc comments #1308)

• Why does xclim.sdba API have a separate section, and it is not in the USER API session? As a user, who wants a description of any functionality of this subpackage, I would go to find it in that section. I think that it's better if all the APIs are in the USER API section.(Contributing and overall doc comments #1308)

About the repository

• In my opinion, I like to see the licence type in the README file likes: (Contributing and overall doc comments #1308)
  ## License This is free software: you can redistribute it and/or modify it under the terms of the Apache License 2.0. A copy of this license is provided in LICENSE.txt.

• The CONTRIBUTING.rst file is in .github/ directory. I suggest moving it to the main directory ./ because it's easier to find it. (Contributing and overall doc comments #1308)

• When I ran the test using make test, only one test failed: (will be resolved in [PyOpenSci] test_mean_radiant_temperature sometimes fails with segfault #1303)
  FAILED xclim/testing/tests/test_atmos.py::TestMeanRadiantTemperature::test_mean_radiant_temperature ===== 1 failed, 1398 passed, 66 skipped, 2 xfailed, 1 xpassed, 95 warnings in 315.63s (0:05:15) ====== make: *** [Makefile:68: test] Error 1

The JOSS paper

• In line number 49 you write: "The development of xclim started at Ouranos in 2018 out of the need to deliver data for a pan-Canadian atlas of climate indicator." . What it is Oranos? Maybe a line about this or a link to the website.

The code

About the code, I don't have any objection. I have only noted that the code is sometimes difficult to follow. So, I just have a few observations that might help to improve this in future developments.

• For example, xclim.ensembles.create_ensemble has an argument called mf_flag. This argument is a bool for definition, so the user can understand that it's a flag. Therefore, this argument can be called multifile to be more readable. (Deprecate mf_flag call signature in xclim.ensembles, expose calendar conversion utilities, fix _gen_returns_section bug #1317)

• Other example is xclim.core.convert_calendar. It has 2 private functions (_yearly_interp_doy, _yearly_random_doy) that could be out of the main function. This makes the if statements easier to follow due to the code has a very large if statement. (Deprecate mf_flag call signature in xclim.ensembles, expose calendar conversion utilities, fix _gen_returns_section bug #1317)

Documentation Checklist

• If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem. (Contributing and overall doc comments #1308)

Code of Conduct

• I agree to follow this project's Code of Conduct

[huard]

Added text in paper to describe Ouranos.