-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a function for integrating SED flux, for different types of radiative processes #153
Conversation
added a test for more realistic data (based on the synchrotron radiation sample from the agnpy documentation); added the method for photon flux SED integral; changed input parameters (allow any equivalency of Hz)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @grzegorzbor, many thanks for this first contribution!
What about making this PR a bit more general?
That is, instead of SedFluxIntegrable
, I think it would be much better to directly introduce a general SedFlux
(or Flux
) class (from which all the radiative processes would inherit) that not only performs the integration of the flux, but also handles all these conversions that we deal with (and keep repeating) in several parts of the package. For example, the class would return the flux either as Gammapy
wrapper and what you would finally like to integrate to obtain the integral flux in
Hi @grzegorzbor, I took the liberty of doing some fixes and making the code a bit slimmer. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #153 +/- ##
==========================================
+ Coverage 96.95% 97.02% +0.06%
==========================================
Files 40 43 +3
Lines 3319 3395 +76
==========================================
+ Hits 3218 3294 +76
Misses 101 101
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The SedFluxIntegrable is a mixin that can be added to all classes that implement sed_flux function.
It provides 2 functions, one for calculating energy flux integral, and the other for photon flux integral, over frequency range.