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

chore: wheel not required for setuptools PEP 517 (all-repos) #79

Merged
merged 4 commits into from
Apr 8, 2022

Conversation

henryiii
Copy link
Member

@jpivarski
Copy link
Member

@aryan26roy, dropping "wheel" from pyproject.toml is no the change I'm asking you to review (@henryiii did it to many of the Scikit-HEP projects at once; read PEP 517 to find out why).

The test failed because of numerical precision. I wrapped everything in pytest.approx (taking care because it doesn't automatically recurse through nested lists), though I'm not sure why you or GitHub Actions hasn't seen that before. If it's failing now, it should have failed before. We might not be able to figure this out, but I just wanted to give you a chance to look at it before merging it.

@henryiii
Copy link
Member Author

henryiii commented Apr 8, 2022

This seems to be trying to run approx on a dict. It does support numpy arrays and lists, but not dicts.

@jpivarski
Copy link
Member

I fixed that once. (Or something like that.) I'll look at it again now.

@jpivarski
Copy link
Member

Oh, and you can run approx on dicts, just not anything deeper.

>>> {"one": 1.1+1e-12, "two": 2.2+1e-12} == pytest.approx({"one": 1.1, "two": 2.2})
True
>>> [{"one": 1.1+1e-12, "two": 2.2+1e-12}] == pytest.approx([{"one": 1.1, "two": 2.2}])
False

The case that's failing in the tests is a list of dict (like above).

@jpivarski
Copy link
Member

I think I have done this before, because the quick fix of deepening everything with list comprehensions with dummy variables x and y is something that I would do. We're seeing this failure because now pytest.approx is looking one level of nesting less than it did before. Are we going backward in pytest versions? I would expect its range of applicability to broaden with time, not narrow.

(In fact, they could implement pytest.approx recursively and make it work for all levels of depth. Oh well.)

@henryiii
Copy link
Member Author

henryiii commented Apr 8, 2022

I think you are looking for dirty-equals?

@henryiii
Copy link
Member Author

henryiii commented Apr 8, 2022

Let's get this in to un-break things.

@henryiii henryiii merged commit cdc7851 into main Apr 8, 2022
@henryiii henryiii deleted the all-repos_autofix_all-repos-sed branch April 8, 2022 17:12
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.

2 participants