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

server route for getpercentile method #798

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Conversation

karishma-gangwani
Copy link
Contributor

@karishma-gangwani karishma-gangwani commented Nov 3, 2023

Description

I tested it with the SJ life dataset by running logistic regression. and all tests passed on my local. There are some types in there that need to be defined better. I think I will get to that afterward after discussing it with Colleen and Edgar. Let me know if this looks good.

Checklist

Check each task that has been performed or verified to be not applicable.

  • Tests: added and/or passed unit and integration tests, or N/A
  • Todos: commented or documented, or N/A
  • Notable Changes: updated release.txt, prefixed a commit message with "fix:" or "feat:", added to an internal tracking document, or N/A

xzhou82
xzhou82 previously approved these changes Nov 3, 2023
Copy link
Collaborator

@xzhou82 xzhou82 left a comment

Choose a reason for hiding this comment

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

looks good and all seem to work

@xzhou82
Copy link
Collaborator

xzhou82 commented Nov 3, 2023

can fix this so all server unit tests will pass?

/home/runner/work/proteinpaint/proteinpaint/server/routes/burden.ts
97:35 error Don't use {} as a type. {} actually means "any non-nullish value".

  • If you want a type meaning "any object", you probably want object instead.
  • If you want a type meaning "any value", you probably want unknown instead.
  • If you want a type meaning "empty object", you probably want Record<string, never> instead.
  • If you really want a type meaning "any non-nullish value", you probably want NonNullable<unknown> instead @typescript-eslint/ban-types

@xzhou82 xzhou82 self-requested a review November 3, 2023 17:48
@karishma-gangwani
Copy link
Contributor Author

can fix this so all server unit tests will pass?

/home/runner/work/proteinpaint/proteinpaint/server/routes/burden.ts
97:35 error Don't use {} as a type. {} actually means "any non-nullish value".

  • If you want a type meaning "any object", you probably want object instead.
  • If you want a type meaning "any value", you probably want unknown instead.
  • If you want a type meaning "empty object", you probably want Record<string, never> instead.
  • If you really want a type meaning "any non-nullish value", you probably want NonNullable<unknown> instead @typescript-eslint/ban-types

thanks for pointing out. I just pushed a fix.

@xzhou82 xzhou82 merged commit ce8b2e7 into master Nov 3, 2023
2 checks passed
@xzhou82 xzhou82 deleted the getpercentile-route branch November 3, 2023 19:29
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.

3 participants