conda-forge / wetterdienst-feedstock

A conda-smithy repository for wetterdienst.
BSD 3-Clause "New" or "Revised" License
3 stars 5 forks source link

Improve runtime dependencies #57

Closed amotl closed 1 year ago

amotl commented 2 years ago

Hi there,

at https://github.com/conda-forge/wetterdienst-feedstock/issues/53#issuecomment-1202058124, @kmuehlbauer said:

I'm having a hard time to understand which are required runtime dependencies of wetterdienst and which are optional. So the requirements.run section in meta.yaml would need some expert eyes on it.

With kind regards, Andreas.

https://github.com/conda-forge/wetterdienst-feedstock/blob/dbcf6e9b3570c7627d86b1846b2627b7f942c6c0/recipe/meta.yaml#L25-L55

amotl commented 2 years ago

Hi again,

from my perspective, that list does not look bad at all. At the first sight, I am not able to identify any wrong item in this list. Maybe @gutzbenj also wants to check and add additional thoughts here?

Cheers, Andreas.

gutzbenj commented 2 years ago

@amotl that list doesn't look bad indeed. We could try to opt-out PyPDF2, as I think it is only used to parse meta information from DWD at this moment.

amotl commented 1 year ago

opt-out PyPDF2, as I think it is only used to parse meta information from DWD at this moment.

But what if that functionality would be needed? Should the user be advised to install the package manually instead? Why are you explicitly picking this module?

A few more general questions come to mind:

amotl commented 1 year ago

Hi again,

finally, https://github.com/earthobservations/wetterdienst/pull/788 will significantly reduce dependencies maybe previously declared to be needed at runtime, best reviewed per https://github.com/earthobservations/wetterdienst/commit/543ceb4f7.

Based on this change, and the next release, we may want to revisit the corresponding conda configuration.

With kind regards, Andreas.

amotl commented 1 year ago

I've just reviewed the list of dependencies again and believe they are good as they are. Feel free to reopen if you think differently.

amotl commented 1 year ago

Hm. Do we want to add those?

Packages found by source code inspection but not in the meta.yaml:

amotl commented 1 year ago

On this admonition, I think we can take those actions.

Packages found in the meta.yaml but not found by source code inspection:

amotl commented 1 year ago

I've submitted #71, which just follows the suggestions by regro-cf-autotick-bot. Let me know what you think about it.

amotl commented 1 year ago

I think we can consider the improvement to be concluded. Thanks for your support, @xylar!