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

Missing coverage? #646

Open
jakirkham opened this issue Nov 17, 2024 · 3 comments · Fixed by #651
Open

Missing coverage? #646

jakirkham opened this issue Nov 17, 2024 · 3 comments · Fixed by #651

Comments

@jakirkham
Copy link
Member

In the recent CodeCov report, they flag these lines as uncoverged

def encode(self, buf):
try:
buf = np.asarray(buf)
except ValueError:
buf = np.asarray(buf, dtype=object)

It is a little unclear why as those lines were introduced with corresponding tests some time ago in PR ( #417 ). AFAICT recent NumPy versions still need to use that fallback. In fact that behavior is expected and defined in NEP 34

This may need further investigation to resolve


Additionally CodeCov flags this line as uncovered.

@pytest.mark.parametrize("codec_class", ALL_CODECS)
def test_docstring(codec_class: type[numcodecs.zarr3._NumcodecsCodec]):
if codec_class.__doc__ is None:
pytest.skip()
assert "See :class:`numcodecs." in codec_class.__doc__

Just looking at it, would naively add a # pragma: no cover, but wanted to mention it here in case there is something I'm missing

@jakirkham
Copy link
Member Author

Thanks David! 🙏

@jakirkham
Copy link
Member Author

jakirkham commented Nov 19, 2024

Reopening as the JSON case is still present

@jakirkham jakirkham reopened this Nov 19, 2024
@jakirkham
Copy link
Member Author

jakirkham commented Nov 19, 2024

This is the test that would ideally cover the JSON case

@pytest.mark.parametrize(
('input_data', 'dtype'),
[
([0, 1], None),
([[0, 1], [2, 3]], None),
([[0], [1], [2, 3]], object),
([[[0, 0]], [[1, 1]], [[2, 3]]], None),
(["1"], None),
(["11", "11"], None),
(["11", "1", "1"], None),
([{}], None),
([{"key": "value"}, ["list", "of", "strings"]], object),
([0], None),
([{'hi': 0}], "object"),
(["hi"], "object"),
(0, None),
],
)
def test_non_numpy_inputs(input_data, dtype):
# numpy will infer a range of different shapes and dtypes for these inputs.
# Make sure that round-tripping through encode preserves this.
data = np.array(input_data, dtype=dtype)
for codec in codecs:
output_data = codec.decode(codec.encode(data))
assert input_data == output_data.tolist()

However we coerce to a NumPy array, which bypasses the try...except... handling above

data = np.array(input_data, dtype=dtype)

So either that test needs to be adapted to trigger the failure or a new test is needed

@slevang slevang mentioned this issue Nov 28, 2024
7 tasks
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 a pull request may close this issue.

1 participant