Repository navigation
cosmo: Hubble parameter trait - #18271
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
@nstarman Have a look at this when you get a chance. |
|
I will fix the tests first tonight |
nstarman
left a comment
There was a problem hiding this comment.
Thanks @preshanth!
I think starting with _HubbleParameter as a private class is the right way to start.
|
@nstarman I think this covers yours suggestions. If you are fine with it I will push some edits for the docs and then this can be converted from draft to PR. |
|
@nstarman I can pick up the next one now that I am warmed up and if you are happy with this. |
nstarman
left a comment
There was a problem hiding this comment.
@preshanth I think this PR is close. The H method needs to be removed from FLRW so that it uses the HubbleParameter.H method that it inherits.
|
I wonder if we have to add tests directly using traits to ensure that the code coverage does not drop. |
|
Sorry this is taking forever. Its a good template for me. Thanks for your efforts @nstarman |
|
@preshanth the CI is picking up an issue in the docs. |
|
On it. |
|
@preshanth because of squash-merging it's not a big deal, but in general we prefer to do rebasing rather than merging to update a PR, e.g. c426c39. Just a PSA :). |
This makes my master diverge from the main right. That throws me off. I can give it a shot next time. |
|
One more docs failure. Same thing in 7.2.rst |
|
@nstarman I finally got all the docs right. The rest should pass. |
nstarman
left a comment
There was a problem hiding this comment.
Thanks @preshanth for the PR! Looks great. Let's get this in.
Description
This pull request is to address the introduction of hubble parameter as a trait in the FLRW cosmology class.
Fixes a portion of #17150