Supernova volumetric redshift rate

[elizabethswann]

Description

Test pull request

Checklist

• Follow the Contributor Guidelines
• Write unit tests
• Write documentation strings
• Assign someone from your working team to review this pull request
• Assign someone from the infrastructure team to review this pull request

[codecov]

Codecov Report

Merging #140 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #140   +/-   ##
=======================================
  Coverage   98.90%   98.90%           
=======================================
  Files          18       18           
  Lines         457      457           
=======================================
  Hits          452      452           
  Misses          5        5           

---

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 e92ed25...ce81b81. Read the comment docs.

[rrjbca]

Looks like you have everything working!

[ghost]

Hello @elizabethswann! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file skypy/supernova/Ia_redshift_distribution.py:

Line 20:1: E302 expected 2 blank lines, found 1
Line 20:80: E501 line too long (95 > 79 characters)
Line 36:80: E501 line too long (96 > 79 characters)
Line 41:1: W293 blank line contains whitespace
Line 44:8: W291 trailing whitespace
Line 45:8: W291 trailing whitespace
Line 46:8: W291 trailing whitespace
Line 48:1: W293 blank line contains whitespace
Line 53:1: W293 blank line contains whitespace
Line 56:1: W293 blank line contains whitespace
Line 58:80: E501 line too long (93 > 79 characters)
Line 73:80: E501 line too long (83 > 79 characters)
Line 76:5: E265 block comment should start with '# '
Line 78:14: E225 missing whitespace around operator
Line 78:80: E501 line too long (103 > 79 characters)
Line 80:21: W292 no newline at end of file

• In the file skypy/supernova/Ia_redshift_rate.py:

Line 44:45: E225 missing whitespace around operator

Comment last updated at 2020-08-29 23:02:23 UTC

[rrjbca]

Looks great thank you. Are you also able to write a couple of short tests for this function? You would need to create:

• skypy/supernova/tests (directory)
• skypy/supernova/tests/__init__.py (empty file)
• skypy/supernova/tests/test_rate.py

And in test_rate.py you would write a function Ia_redshift_rate that runs a few simple tests. Perhaps look at test_size.py for inspiration. E.g. you could write a test that checks the units of the output are correct

[rrjbca]

I think this whitespace will fix your PEP8 error. Also I think it should be units.year ** -1 is that correct?

Suggested change

	rate = r0*(1+redshift)**a * units.year *(units.Mpc)**-3
	rate = r0 * (1+redshift) ** a * units.year ** -1 * (units.Mpc) ** -3

[rrjbca]

Wondering what are these default values? Are they "standard" values? I would generally prefer them to not have defaults in most cases. They should also be documented under 'Parameters' even if it just says 'free parameters of the model'

[rrjbca]

Will probably have to update the units here as well

[itrharrison]

Closed in favour of #540 which targets the correct branch of module/supernovae