cosimoNigro / agnpy

Modelling jetted Active Galactic Nuclei radiative processes with python
https://agnpy.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
48 stars 33 forks source link

Fix gammapy and sherpa imports #125

Closed cosimoNigro closed 2 years ago

cosimoNigro commented 2 years ago

This PR is a followup of issue #122.

I thought that adding gammapy and sherpa to the install_requires of conf.py would solve the issue. Instead when I try to create a distribution file and install it, the dependencies cannot be resolved via pip, that fails to install sherpa. The problem is not there if I install the packages with mamba after installing the dist file.

Therefore, following the initial suggestion of @jsitarek, I added a try/except clause in agnpy/fit/__init__.py printing a warning message if the import fails. The rest of the modules can be used then, even without gammapy and sherpa.

I added a clarification in the docs suggesting to install the fitting packages separately if agnpy is installed via pip or conda.

@jsitarek can you take a quick look?

Solves #122.

codecov[bot] commented 2 years ago

Codecov Report

Merging #125 (c53c92b) into master (a272d5d) will decrease coverage by 0.09%. The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
- Coverage   97.43%   97.33%   -0.10%     
==========================================
  Files          38       38              
  Lines        2997     3000       +3     
==========================================
  Hits         2920     2920              
- Misses         77       80       +3     
Flag Coverage Δ
unittests 97.33% <57.14%> (-0.10%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
agnpy/fit/__init__.py 57.14% <57.14%> (-42.86%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

cosimoNigro commented 2 years ago

It seems to be working for the tests I could do, so I would merge and make the release.