Skip to content
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 quantile regression example #608

Merged
merged 4 commits into from
Dec 8, 2022
Merged

Conversation

aloctavodia
Copy link
Collaborator

Closes #593

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -0,0 +1,621 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: change the order of imports


Reply via ReviewNB

@@ -0,0 +1,621 @@
{
Copy link
Collaborator

@tomicapretto tomicapretto Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first paragraph says "Normal" and the second says "normal".

Then I would use "For example, in medical research, pathologies or potential health risks occur at high or low quantiles, for instance, overweight and underweight." (too many commas, but I think it's clearer).

"In some other fields like ecology quantile" -> " In some other fields like ecology, quantile"


Reply via ReviewNB

@@ -0,0 +1,621 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"assymetric paramter" -> "assymetry parameter". After that, I would add a colon ":" or a period ".", but not a comma ",".

Also, does the "kappa" have an interpretable meaning? Asking because honestly I don't know


Reply via ReviewNB

Copy link
Collaborator

@tomicapretto tomicapretto Dec 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw a typo: "There is at least two alternative parameterizations" -> "There are..."

@@ -0,0 +1,621 @@
{
Copy link
Collaborator

@tomicapretto tomicapretto Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a formula showing like this

$$

\kappa = (\frac{q}{1 - q}) ^ 0.5

$$

So it looks like it's the square root of the logit of q? I don't know if it adds any value, I just realized about it


Reply via ReviewNB

@@ -0,0 +1,621 @@
{
Copy link
Collaborator

@tomicapretto tomicapretto Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add one more sentence saying the assymetriclaplace family allows the model to account for the increased variability in BMI as the age increases, while for the Gaussian family that variability always stays the same.


Reply via ReviewNB

@tomicapretto
Copy link
Collaborator

Overall it looks really good. I would consider the following points

  • Blackify the code
  • Add labels to the plots

@codecov-commenter
Copy link

Codecov Report

Merging #608 (2e89a5f) into main (a53a3eb) will not change coverage.
The diff coverage is n/a.

❗ Current head 2e89a5f differs from pull request most recent head 1ff04fc. Consider uploading reports for the commit 1ff04fc to get more accurate results

@@           Coverage Diff           @@
##             main     #608   +/-   ##
=======================================
  Coverage   82.84%   82.84%           
=======================================
  Files          38       38           
  Lines        3026     3026           
=======================================
  Hits         2507     2507           
  Misses        519      519           

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

@aloctavodia aloctavodia merged commit 4475413 into bambinos:main Dec 8, 2022
@aloctavodia aloctavodia deleted the qre branch December 8, 2022 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add quantile regression example
3 participants