-
Notifications
You must be signed in to change notification settings - Fork 16
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
Test latest dependencies #344
Changes from 29 commits
b7daa56
0b549bd
5d53524
16f0ff0
8edb717
8a60ee8
e5ec305
e5032f3
b3434b1
5b25b87
8c1e854
f4bdb46
5db5ae9
80289c9
9306b31
636bcbf
c59b5c1
1aae3dc
0eef161
8cf9914
77c3de9
f245a9a
f1aefb3
00c67dd
f1d796f
c11fbc1
83e00dd
516683e
2c52c6e
bec343f
4cefb0d
33ceb6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,15 +151,66 @@ jobs: | |
with: | ||
python-version: 3.9 | ||
|
||
- name: Install dependencies | ||
- name: Upgrade pip and install tomli | ||
run: | | ||
python -m pip install --upgrade pip | ||
python -m pip install tomli | ||
eval "python -m pip install $(./bin/min_dependencies.py)" | ||
python -m pip install -e ".[test]" | ||
- name: Install minimum dependencies | ||
run: eval "python -m pip install $(./bin/min_dependencies.py)" | ||
- name: Install anesthetic | ||
run: python -m pip install -e ".[test]" | ||
|
||
- name: Test with pytest | ||
run: python -m pytest --cov=anesthetic tests | ||
|
||
- name: Upload coverage to Codecov | ||
uses: codecov/codecov-action@v1 | ||
|
||
latest-dependencies: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Set up latest stable Python 3 | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: 3.x | ||
|
||
- name: Upgrade pip and install tomli | ||
run: | | ||
python -m pip install --upgrade pip | ||
python -m pip install tomli | ||
- name: Install latest dependencies | ||
run: eval "python -m pip install $(./bin/latest_dependencies.py)" | ||
- name: Install pytest and anesthetic | ||
run: | | ||
python -m pip install pytest | ||
python -m pip install --no-deps -e . | ||
|
||
- name: Test with pytest | ||
run: python -m pytest | ||
|
||
- name: Upload coverage to Codecov | ||
uses: codecov/codecov-action@v1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to upload to codecov for this job? If we don't then I'd prefer to leave this out. Faster and easier to review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should have flagged this earlier as I wasn't sure myself. @williamjameshandley what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the github settings will also need adjusting to control which tests are required to pass in order for a PR to be merged. I don't think I have access to these settings, @lukashergt can you check them? Maybe these tests will need to be put in a different file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are only branch protection rules for the master branch, so until this PR gets merged there is nothing to be done. Once this is merged we can individually remove jobs from the master branch protection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are all the jobs under CI separate or one in the settings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separate. Each job can be added/removed individually. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lgim then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
??? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "let's get it merged", which I think is what @williamjameshandley thought I meant by "lgtm" (looks good to me, which I learned from pandas) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yeah, we had this before... I don't retain this stuff...^^ |
||
|
||
check-for-new-versions: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- name: Set up latest stable Python 3 | ||
uses: actions/setup-python@v4 | ||
with: | ||
python-version: 3.x | ||
|
||
- name: Upgrade pip and install tomli and requests | ||
run: | | ||
python -m pip install --upgrade pip | ||
python -m pip install tomli requests | ||
|
||
- name: Install anesthetic | ||
run: python -m pip install -e ".[test]" | ||
|
||
- name: Check anesthetic dependencies are up to date | ||
run: python -m pytest ./bin/check_up_to_date.py | ||
|
||
- name: Upload coverage to Codecov | ||
uses: codecov/codecov-action@v1 | ||
lukashergt marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
__version__ = '2.4.1' | ||
__version__ = '2.4.2' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import pytest | ||
import requests | ||
from packaging import version | ||
import importlib | ||
from latest_dependencies import deps | ||
|
||
|
||
packages = [importlib.import_module(package) for package in deps] | ||
|
||
|
||
@pytest.mark.parametrize('package', packages) | ||
def test_packages(package): | ||
response = requests.get(f"https://pypi.org/pypi/{package.__name__}/json") | ||
latest_version = response.json()['info']['version'] | ||
|
||
if version.parse(latest_version) > version.parse(package.__version__): | ||
print(f"You should upgrade the {package.__name__} requirement " | ||
f"from {package.__version__} to {latest_version}") | ||
assert version.parse(latest_version) <= version.parse(package.__version__) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#!/usr/bin/env python | ||
import tomli | ||
|
||
with open("pyproject.toml", 'rb') as f: | ||
pyproject = tomli.load(f) | ||
|
||
deps = pyproject["project"]["dependencies"] | ||
deps = [dep.partition("==")[0] for dep in deps] | ||
deps = [dep.partition(">=")[0] for dep in deps] | ||
deps = [dep.partition("<=")[0] for dep in deps] | ||
deps = [dep.partition(">")[0] for dep in deps] | ||
deps = [dep.partition("<")[0] for dep in deps] | ||
deps = [dep.partition("~=")[0] for dep in deps] | ||
deps = [dep.partition("^=")[0] for dep in deps] | ||
|
||
if __name__ == "__main__": | ||
deps = [f'"{dep}"' for dep in deps] | ||
print(' '.join(deps)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the codecov upload for
minimum-dependencies
, too? Not really what this job is about either, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, since it is necessary that anesthetic works with the lowest versions we allow, and a PR should not be merged unless this is true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But codecov has nothing to do with whether anesthetic works or not, that is what pytest is for. Codecov should definitely be in the pip and conda jobs (or at least in one of them), but the coverage does not really depend on versions (unless we specifically add version related conditional statements to the code, which we don't). Besides, the
minimum-dependencies
job does not even include extra dependencies, so it doesn't do a full code coverage anyhow...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the
minimum-dependencies
code coverage were to fail, while the pip job code coverage passes, then we wouldn't even know about it, because as long as the pip jobs cover every line at least once, codecov will mark 100% coverage. But I don't even think it is possible for code coverage to be different for the pip jobs and the minimum job...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the tests are now different for matplotlib≥3.8, and at one point in beta some of the anesthetic plotting code was different for pandas 1 vs 2. cov would tell us when these lines are no longer needed right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try it, but I think there will be missing coverage without
minimum-dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code in
tests/
doesn't matter for code coverage, only code inanesthetic/
matters.In a situation like that we would indeed need the code coverage in
minimum-dependencies
for as long as we wanted to be backwards compatible. However, we already have pandas>=2.0.0, so this shouldn't be an issue, here.In fact, I think this might be an argument in favour of not having code coverage in
minimum-dependencies
, to flag up once parts of code are no longer needed for newer versions. Currently we wouldn't know if parts of code were no longer needed by newer versions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it depends how cut-throat we want to be with older packages. I suppose the pandas2 difference was because pandas2 hadn't actually released yet, and once it did, we dropped support for pandas1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - I'd thought
pytest --cov anesthetic tests
meant that coverage was looking at anesthetic AND tests, but tests is just to tell pytest where the tests are. I don't think I specified this in the new pytest callsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that there might be a point where we'll want to re-introduce this for backwards compatibility, but I'd rather be aware when that happens.