Feature/extras require, closes #165

[caldwellst]

Monica! Putting this as draft because I have put lots of changes in here to make it run in the end, but I hope the functionality makes it through in some form.

So in general, my approach was to find a way to add extra requirements to the setup.cfg file. On the way, however, I have made some additional changes to make this happen. Here’s a general list of what I’ve done to guide your review.

1. By adding in the sphinxcontrib.extras_require section to setup.cfg, I started to explore the best way to reference files and install dev, doc, or whatever requirements. It seemed that incorporating all of his into setup.cfg was simpler than the mixed requirements files. I have tested and it seems to all work, and is cleaner, but maybe I’m missing some catch.
2. As part of this, I removed all requirements files, because it seems we can rely on pip to sort out dependencies however we install the package! Does that make sense?
3. Finally, I think that means pip-tools was no longer required for dev. My testing didn’t show any problems getting rid of it, but not sure of that.
4. Then I got ambitious. I discovered that you can automatically document the required extras using some sphinx references with .. extras-require:. However, for that to work it required that the sphinx base directory was only one level away from setup.cfg. Thus for the test to show what I’ve done, I put docs-source and docs-build separately within the main folder. But I know this might not be great in a final way, so just wanted to put that here for now for your draft review.
5. I did a quick check and I think GLOFAS is the only optional dependencies we should have now, but wasn't thorough, so we can add more before we merge if you like this.

[github-actions[bot]]

Unit Test Results

    5 files  ±  0      5 suites  ±0   39s :stopwatch: +2s   95 tests +  4    95 :heavy_check_mark: +  4  0 :zzz: ±0  0 :x: ±0  475 runs  +20  475 :heavy_check_mark: +20  0 :zzz: ±0  0 :x: ±0 

Results for commit ce34fcc6. ± Comparison against base commit 57533923.

:recycle: This comment has been updated with latest results.

[codecov-commenter]

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@400dd87). Click here to learn what that means. The diff coverage is 85.71%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             develop     #167   +/-   ##
==========================================
  Coverage           ?   90.70%           
==========================================
  Files              ?       20           
  Lines              ?     1324           
  Branches           ?        0           
==========================================
  Hits               ?     1201           
  Misses             ?      123           
  Partials           ?        0           

Impacted Files	Coverage Δ
src/ochanticipy/datasources/chirps/chirps.py	81.86% <ø> (ø)
src/ochanticipy/datasources/glofas/forecast.py	97.05% <ø> (ø)
...hanticipy/datasources/iri/iri_seasonal_forecast.py	93.50% <ø> (ø)
src/ochanticipy/datasources/glofas/glofas.py	90.18% <66.66%> (ø)
src/ochanticipy/utils/check_extra_imports.py	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[caldwellst]

I think it's ready for review now! Took me a while to sort out exactly what approach to take for the optional dependencies, here were some specifics I dealt with:

1. Module-level imports of an optional dependency is impossible, since that generates errors any time you run import aatoolbox if the dependency is unavailable.
2. Rather than import in __init__, in which case I think I would've had to make cdsapi a property of the Glofas class, I just imported directly in the _download() method. However, I run the checks directly in __init__. Seems this might be a good model for any other optional dependencies going forward unless they are used across all methods.
3. For testing, I realized that it took me a while to ensure that the library doesn't error out if you don't have all dependencies. This is all based on the module-level implementation in Glofas rather than the checks. Thus I think each module with optional dependencies needs to specifically test that we can import aatoolbox without error if their dependencies aren't available, and that the correct error is generated if they are available.

Look forward to your review and hope the approach is logical!

[github-actions[bot]]

Test Results

    4 files  ±  0      4 suites  ±0   30s :stopwatch: ±0s   97 tests +  4    97 :heavy_check_mark: +  4  0 :zzz: ±0  0 :x: ±0  388 runs  +16  388 :heavy_check_mark: +16  0 :zzz: ±0  0 :x: ±0 

Results for commit add5ba28. ± Comparison against base commit b93f6ce7.

:recycle: This comment has been updated with latest results.

[caldwellst]

Alright! I've convinced pip-tools to use our new extras setup and recreated requirements-dev.txt. I didn't recreate the other files because now things like RTD can just install from the setup.cfg. I have also put things into a single doc folder with a docs/_build to store the build file and updated the contributing file. I think as you say it makes things cleaner in the repository and also isn't that confusing for the end user I think! It's also the default setup of the constructor from sphinx-build if you get a template (that's why it's setup that way in calendar) so I think is also "recommended" let's say.

I have merged in with develop and looks like it's ready to merge! Let me know what you think now.

[caldwellst]

Absolutely no worries, I love this stuff and having fun going back through it. Apologies, I think I lost my mind at some point and removed some documentation and then got super busy and who knows what happened. I've added that stuff back. I still have a few changes to make to documentation, but wanted to first add this discussion point on requirements.

Requirements files

Now, onto the debate of the requirements and how we address them. I have looked into pip-compile-multi, and here are my thoughts.

The benefits of pip-compile-multi:

• Ability to compile multiple requirements files together and address mismatched dependencies.
• Streamlined pre-commit since you simply need to have requirements files stored as .in files and it automatically takes care of the rest.

The cons of pip-compile-multi:

• Large number of requirements files instead of storing in one place together, see flask for example.
• It doesn't include the optional dependencies of flask automatically. For flask, the user is required to manually install them. You would need to add individual .in files and then have a full.in file that would create the way to install the full package.

The benefits of extras_requires:

• Ability to store all package requirements and extras in a single config (in this case setup.cfg), so basically streamlines the requirements files system
• With sphinx, can then easily reference the extra requirements for specific modules of our package, as I've done with glofas
• Users can install the full package or optional extras with pip install ocha-anticipy[full] or pip install ocha-anticipy[glofas] without having to manually install dependencies, which I think would scale well.

The cons of extras_requires:

• You don't have the streamlined pre-commit, need to manually build all the .txt files we want.
• Lack of the pip-compile-multi dependency resolution across multiple .txt file requirements.

My vote

Now, my understanding though is that requirements-dev.txt should have consolidated all dependencies necessary for production, testing, documentation, and development, since we have sourced the entirety of the package in there. Does pip-compile-multi actually provide any benefit on top of this? It might, as I'm not still totally clear on what it does.

However, I would vote for extras_require because it means we can streamline installation of optional extras to the user and add in the documentation with sphinx. It also maintains all the extras in the setup.cfg which in my mind is cleaner and easier to parse than having separate .in files to manage (although that's not actually that complex).

Other question

My other question is, you asked to build the other requirements files, so I did. But I'm wondering, why are we doing that? Why don't we just need a development only requirements file that we use to develop from? Is it just in case we wanted to have a system only do documentation or only do testing we produce those? Since end users install from pip, why do we need all the different versions and not just the one dev version?

This is why I only built dev before.