Add support for satpy.composites entry points

[mraspaud]

This adds support for entry_points in satpy, in particular for composites.
The composites defined in other packages (in yaml files) are now loaded directly into satpy.

An example package to achieve this is on display here: https://github.com/mraspaud/satpy-composites-plugin-example

• Tests added
• Tests passed
• Passes flake8 satpy
• Fully documented

[ghost]

Congratulations 🍻. DeepCode analyzed your code in 0.378 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.

---

☺️ If you want to provide feedback on our bot, here is how to contact us.

[djhoese]

Just to make sure these are connected, this is related to #784 and #789. Nice job getting an example going.

I think this would be good as a short term solution, but I'm not sure a short term solution will evolve into a long term solution for such an involved interface like this. My biggest concern is with the discussion that we had in the referenced issues about data_files versus package_data. I'm not sure using an etc directory as package_data is the "correct" way of doing this. If we used data_files we could put things in the environments <prefix>/etc/satpy directory. I'm not sure what that would mean for plugin packages so I admit that this PR's solution may still be best.

Another question, in the setup.py for the example project you have:

'example_composites = satpy_cpe',

Is there any chance of packages overriding other packages if they both use example_composites?

[gerritholl]

Wouldn't external packages putting things in <prefix>/etc/satpy cause collisions? Fogpy has etc/fogpy/composites/abi.yaml and so does Satpy.

[coveralls]

Coverage decreased (-0.01%) to 89.314% when pulling 7a615cb on mraspaud:feature-composite-entry-points into ec24bcc on pytroll:master.

[coveralls]

Coverage increased (+0.008%) to 89.336% when pulling 3b2e6f5 on mraspaud:feature-composite-entry-points into ec24bcc on pytroll:master.

[mraspaud]

@djhoese thanks for provided the quick feedback and reference to previous issues.

This doesn't indeed change anything to the way composites are loaded, it merely adds extra config paths to the places that are searched for composite configs. But this can be update when/if we change to directory structures.

I don't really like the idea of having all configs in the same central satpy directory, risks of conflicts are present. And it would be clearer IMO to keep in each plugins structure. (And how would uninstallation be handled?)

Regarding your last question, the example_composites key doesn't need to be unique. I tried creating a second toy plugin and got the following entry points:

[EntryPoint.parse('example_composites = satpy_cpe'),
 EntryPoint.parse('example_composites = satpy_cpe2')]

[djhoese]

@mraspaud sounds good on the example_composites stuff.

@gerritholl @mraspaud for the <prefix>/etc/satpy, if all packages put their stuff in that directory then yes it could be an issue. The idea in the previous issues was that Satpy would load all config files found instead of just specific sensors so conflicts would be less likely.

That said, we could also expect <prefix>/etc/<plugin_pkg>/ directories or <prefix>/etc/satpy/<plugin_pkg>/ directories. I'm not pushing too hard on that, but it is a possibility.

[mraspaud]

Ok, thanks for clarifying.
I think <plugin_pkg> isn't so nice, because I'd rather have all package have the same path to the config files.

So, you are proposing to use etc/satpy/composites/viirs/snow_composites.yaml for example right ?
(why add satpy in there ?)
I agree this would be a good thing, but do we need this in this PR ? We will anyway have to have a transition/deprecation period as people already now have their own custom version of the config structure they use...

[djhoese]

So, you are proposing to use etc/satpy/composites/viirs/snow_composites.yaml for example right ?
(why add satpy in there ?)

I'm not sure on the full path. For the satpy prefix, this is going in <prefix>/etc/satpy along side everything else in your normal etc directory. This is outside the satpy package. So if you installed this in a conda environment it would be in ~/miniconda3/envs/satpy_env/etc/. We'd want to distinguish our configs from every other package/library installed in an environment.

but do we need this in this PR ?

My original concern/point was do we want to take the shortcut for this solution only to possibly regret it later. Meaning, if we want to do "data_files" or restructure the etc directory inside satpy later rather than before this PR, it could effect a lot of users.

That said, I'm leaning towards just merging this.

[mraspaud]

OK, then I think this requires more discussion. I'll still add some tests so we can merge this now. If we mark this as experimental, we're got our backs free 🙂

[djhoese]

So maybe we keep it undocumented until @gerritholl and @jsolbrig can test it out in the real world?

[mraspaud]

Yes, something like that. I might start with some light doc, just to make sure we understand each other, but I'll mark it clearly as not to be used yet.

[codecov]

Codecov Report

Merging #1117 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1117      +/-   ##
==========================================
+ Coverage   89.32%   89.33%   +<.01%     
==========================================
  Files         197      197              
  Lines       28999    29022      +23     
==========================================
+ Hits        25904    25927      +23     
  Misses       3095     3095

Impacted Files	Coverage Δ
satpy/composites/__init__.py	78.78% <100%> (+0.04%)	⬆️
satpy/tests/test_config.py	91.13% <100%> (+1.58%)	⬆️
satpy/config.py	83.47% <100%> (+1.32%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec24bcc...3b2e6f5. Read the comment docs.

[mraspaud]

@djhoese Can we merge this ?

[djhoese]

Go for it. I'll let you do the actual merge in case you needed to change anything.

[mraspaud]

I'll let it be as it is for now.