search

CDCgov / multisignal-epi-inference

Python package for statistical inference and forecast of epi models using multiple signals
https://cdcgov.github.io/multisignal-epi-inference/
10 stars 1 forks source link

Upx3 typos fix #121

Closed gvegayon closed 3 weeks ago

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.77%. Comparing base (48b7471) to head (a59b744). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #121 +/- ## ======================================= Coverage 94.77% 94.77% ======================================= Files 39 39 Lines 842 842 ======================================= Hits 798 798 Misses 44 44 ``` | [Flag](https://app.codecov.io/gh/CDCgov/multisignal-epi-inference/pull/121/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/CDCgov/multisignal-epi-inference/pull/121/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov) | `94.77% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CDCgov#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gvegayon commented 1 month ago

OK, so I was able to update the branch to main (it had lots of conflicts). Checks pass, except for typos, reporting a false positive (see here). I don't think pre-commit should break if this happens. We can discuss further next sprint.

cc @dylanhmorris @AFg6K7h4fhy2 @natemcintosh @damonbayer

gvegayon commented 1 month ago

Also, it is worthwhile mentioning we have to be mindful about keeping PR branches up to date with main. Having to manually deal with conflicts can be avoided.

gvegayon commented 3 weeks ago

@AFg6K7h4fhy2, I just merged main into this to see how it behaves. Depending on that I'll approve and merge :)! Thanks for your patience with this!

gvegayon commented 3 weeks ago

There are still some corrections to do: load should be lod

image

The docstring contains information about the original dataset.

gvegayon commented 3 weeks ago

@AFg6K7h4fhy2, I tried to add regex to capture jnp.arange (which typos tries to correct to jnp.arrange), and it is not working properly. That's what's causing pre-commit to fail.

cc @damonbayer

AFg6K7h4fhy2 commented 3 weeks ago

Do you know if arange is the last failure with the typos PR?

All that needed to be done was adding arange to _typos.toml.

gvegayon commented 3 weeks ago

Thanks, @AFg6K7h4fhy2! But I think the "arange" should be flagged as a typo. I was trying to flag the more specific case of jnp.arange or np.arange as OK. IMO, adding arange to the list of OK words is fine for the moment, but typos should allow us to be more specific.