Codecov Report

Merging #16 (ea6ca6e) into main (b6374b4) will increase coverage by 2.08%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   95.76%   97.85%   +2.08%     
==========================================
  Files           9       11       +2     
  Lines         260      280      +20     
==========================================
+ Hits          249      274      +25     
+ Misses         11        6       -5

Impacted Files	Coverage Δ
bayes_kit/__init__.py	`100.00% <100.00%> (ø)`
bayes_kit/autocorr.py	`100.00% <100.00%> (ø)`
bayes_kit/ess.py	`100.00% <100.00%> (+9.25%)`	:arrow_up:
bayes_kit/iat.py	`100.00% <100.00%> (ø)`

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

bob-carpenter commented 1 year ago

This is ready to re-review. I got up to 100% test coverage, which was really useful for making me be more careful defining and testing boundary conditions. I renamed the "internal" function _end_pos_pairs after @magland pointed out the first name misspecified the function, then documented the boundary conditions, then fixed the function to match the tests. Edit: it also adds anti-correlated tests in a broader range of anti correlations.

bob-carpenter commented 1 year ago

Also, this will close issues #15 and #13 and #3 (I added a new issue for the ranked versions and updated #3).

WardBrian commented 1 year ago

I'm not sure if you'd like to fix them here or in #10 or somewhere else, but I just wanted to note that mypy --strict . raises 61 errors in this branch. It's about 25 between ess.py, test_ess.py and test_autocorrelation.py, with the rest in the RHat code you don't really touch here

bob-carpenter commented 1 year ago

I'l fix the mypy tests. Sorry for skipping bits of the workflow.

Would there be a way to automate (a) black formatting, (b) strict mypy testing, and (c) anything else that's going to cause a PR to be rejected if I forget to do it manually?

WardBrian commented 1 year ago

I plan on automating that as soon as our current main branch would pass it, but a lot of the type errors in these files are already checked in there so it wouldn’t be feasible yet

bob-carpenter commented 1 year ago

Another question on doc. The extra line space is because emacs thinks the first line of doc should fit on a single line. Should I be forcing the first sentence of doc to fit on a single line? Is it OK if it runs over? black didn't reformat it, but emacs doesn't like it.

WardBrian commented 1 year ago

I believe the standard is if it takes more than one line, you should add a newline after the opening """

e.g.


"""A single line docstring"""

"""
A docstring which is much longer, taking up 
multiple lines or even including multiple sections
"""

Black doesn't touch docstrings, but the python emacs package might have this opinion built-in

bob-carpenter commented 1 year ago

Thanks. The examples for the Google-style doc start right after the """ but they're all one-liners. I can try to be more terse if that's the standard in Python.

WardBrian commented 1 year ago

I wouldn’t sacrifice the level of detail you want to avoid the extra newline. In any programmatic usage (e.g. by Sphinx) leading whitespace is removed from them

bob-carpenter commented 1 year ago

I just want to follow conventions which looks like terse first sentences. That's what javadoc wanted, too. I'll see what I can do.

flatironinstitute / bayes-kit

Fix ess imse update #16

Codecov Report