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

Remove unused array_compare tests #515

Merged
merged 4 commits into from
Nov 16, 2023
Merged

Conversation

larrybradley
Copy link
Member

In pytest >7.2 test functions can only return None.
Followup to #480

@astrofrog
Copy link
Member

It's possible to fix this though, we fixed it in pytest-mpl and I need to copy over the fixes to pytest-arraydiff

@larrybradley
Copy link
Member Author

@astrofrog Great! Let me know when there's a new pytest-arraydiff release with the fix.

@astrofrog
Copy link
Member

@larrybradley - could you try the latest dev version of pytest-arraydiff? Just merged a PR fixing this. Thanks!

@larrybradley
Copy link
Member Author

@astrofrog I've updated this PR to use pytest-arraydiff from main. The good news is the tests do not fail. But the bad news is that I changed some of the reference data and even removed an entire file (see the last commit in this PR) and the tests still passed. So it does not appear to be really compare the test output with the reference data.

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Did this used to work as intended in older pytest?

I look at https://github.com/astropy/pytest-arraydiff/blob/main/tests/test_pytest_arraydiff.py and none of the tests are stack with parametrize. And reference_dir is explicitly provided there.

Copy link
Member Author

Choose a reason for hiding this comment

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

According to the README file (https://github.com/astropy/pytest-arraydiff#using), reference is the default directory name.

@larrybradley larrybradley force-pushed the array-cmp branch 2 times, most recently from 533872b to dc2b03d Compare November 16, 2023 03:12
@larrybradley
Copy link
Member Author

larrybradley commented Nov 16, 2023

The --arraydiff option was missing from pytest. Running pytest locally works, but strangely tox does not -- it cannot find the reference data (https://github.com/astropy/regions/actions/runs/6885867875/job/18730669107?pr=515).

Also, now if I do not add the --arraydiff option, pytest fails because the function does not return None.

@pllim
Copy link
Member

pllim commented Nov 16, 2023

@larrybradley , does that mean pytest-arraydiff is working as expected or there is something else to be fixed? Hope you can clarify. Thanks!

@larrybradley
Copy link
Member Author

It seems pytest-arraydiff is working locally as expected when running pytest directly. However, it fails when running the tests with tox (even though tox is running the same pytest command). tox cannot find the reference data for comparison. Is there some path setting or something that needs to be added with tox?

@larrybradley
Copy link
Member Author

This PR should be ready to go, but the (tox) tests are failing because the reference data is not found: https://github.com/astropy/regions/actions/runs/6885867875/job/18730669107?pr=515

@larrybradley
Copy link
Member Author

larrybradley commented Nov 16, 2023

I'm wondering now if the reference data isn't being packaged.. That's not it, the reference data are included.

@pllim
Copy link
Member

pllim commented Nov 16, 2023

Well, tox is supposedly isolated and whatever, so maybe you do need to explicitly point to the reference path even though it is supposedly the default location.

@larrybradley
Copy link
Member Author

@astrofrog -- Any idea why tox cannot find the reference data? I took a look at reproject for reference and I don't see what's wrong here.

@larrybradley
Copy link
Member Author

I fixed the issue -- there was a typo in pyproject.toml package-data. 🤦

@larrybradley larrybradley merged commit a40a3ed into astropy:main Nov 16, 2023
20 checks passed
@larrybradley larrybradley deleted the array-cmp branch November 16, 2023 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants