Test latest dependencies #344

AdamOrmondroyd · 2023-09-24T10:44:56Z

Description

Add upper limits to pandas and matplotlib versions, and add an additional test which uses the most recent releases.

Also increase the README python version to 3.8+

Fixes #343

Checklist:

I have performed a self-review of my own code
My code is PEP8 compliant (flake8 anesthetic tests)
My code contains compliant docstrings (pydocstyle --convention=numpy anesthetic)
New and existing unit tests pass locally with my changes (python -m pytest)
I have added tests that prove my fix is effective or that my feature works
I have appropriately incremented the semantic version number in both README.rst and anesthetic/_version.py

codecov · 2023-09-24T10:52:03Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (434d97e) 100.00% compared to head (33ceb6a) 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #344   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         2868      2868           
=========================================
  Hits          2868      2868

Files	Coverage Δ
anesthetic/_version.py	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AdamOrmondroyd · 2023-09-24T21:07:27Z

First pass done. Will the test become required if this PR was merged? Is there a way to avoid that?

lukashergt · 2023-09-27T23:49:56Z

First pass done. Will the test become required if this PR was merged? Is there a way to avoid that?

This can be configured in the GitHub settings.

AdamOrmondroyd · 2023-10-01T16:07:54Z

Once #339 is done I'll update this accordingly

@williamjameshandley your gymnastics with version numbers in #335 has confused me even more how the test requirements work...

AdamOrmondroyd · 2023-10-01T16:15:54Z

just spotted a mistake in the README which imo fits in with the theme of this PR

lukashergt

There is an issue with this idea, which is that if the next minor pandas/matplotlib releases and doesn't cause any problems, there's nothing to notify us that we should increment the anesthetic requirements

How about we add a test that runs after the latest dependencies test and checks whether the latest pandas and matplotlib minor versions are greater than our listed dependency?

As mentioned in #343, let's add another job that checks whether our matplotlib and pandas version requirements are up to date.

README.rst

.github/workflows/CI.yaml

… pyproject.toml)

.github/workflows/CI.yaml

This reverts commit c59b5c1.

This reverts commit 0eef161.

AdamOrmondroyd · 2023-10-01T23:52:12Z

Problem is this doesn't tell us where the problem is

lukashergt · 2023-10-02T05:14:44Z

Problem is this doesn't tell us where the problem is

Yeah, a check with pytest would probably be more helpful for reviewing, i.e. install with latest versions and then use pytest to make a version check against the versions listet in the dependencies.

AdamOrmondroyd · 2023-10-02T08:05:01Z

It would have to be separate from the rest of the tests, as this is not required to pass

This reverts commit 00c67dd.

AdamOrmondroyd · 2023-10-02T23:13:46Z

I'm quite a bit happier with this version. Currently includes an explicit list of [pandas, matplotlib], @lukashergt would you also like this to be automated? I could probably recycle bin/latest_dependencies.py

…lved)

This reverts commit 516683e.

AdamOrmondroyd · 2023-10-02T23:35:48Z

Now automated to include all basic dependencies. Does come at the expense of bin/latest_dependencies.py involving a little more gymnastics

lukashergt

Nice, just a question inline whether we could avoid the uploading to codecov, if not, then I'm happy with this.

lukashergt · 2023-10-04T00:45:29Z

.github/workflows/CI.yaml

+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v1


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.

I should have flagged this earlier as I wasn't sure myself. @williamjameshandley what do you think?

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 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.

Are all the jobs under CI separate or one in the settings?

Separate. Each job can be added/removed individually.

"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)

Ah, yeah, we had this before... I don't retain this stuff...^^

.github/workflows/CI.yaml

AdamOrmondroyd · 2023-10-04T19:20:25Z

Fyi python 3.12 released a couple of days ago, and the tests are all passing fine (no new warnings anyway!). There's also a new m1 mac github runner we might want to run tests on https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/

Edit: this seems to be larger runners needing a bit more horsepower. I'm not sure what architecture the macos-latest runners are

lukashergt · 2023-10-04T19:33:24Z

.github/workflows/CI.yaml

      - name: Upload coverage to Codecov
        uses: codecov/codecov-action@v1


Should we remove the codecov upload for minimum-dependencies, too? Not really what this job is about either, right?

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

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...

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...

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?

I'll try it, but I think there will be missing coverage without minimum-dependencies

Some of the tests are now different for matplotlib≥3.8,

Code in tests/ doesn't matter for code coverage, only code in anesthetic/ matters.

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?

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.

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?

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.

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.

Some of the tests are now different for matplotlib≥3.8,

Code in tests/ doesn't matter for code coverage, only code in anesthetic/ matters.

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 calls

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.

lukashergt

Thanks a bunch @AdamOrmondroyd! Feel free to squash and merge.

AdamOrmondroyd · 2023-10-04T21:27:42Z

@lukashergt done, please can you check that the PR settings are adjusted accordingly?

lukashergt · 2023-10-04T21:46:44Z

New CI jobs are still not part of branch protection, so PRs won't require these. I guess new jobs need to be added manually to the branch protection rules.

AdamOrmondroyd · 2023-10-04T21:47:40Z

Cool, thanks for checking

lukashergt · 2023-10-04T21:49:52Z

Thanks for taking charge of this, this makes anesthetic a lot friendlier to users other than ourselves.

add latest_dependencies.py and workflow

b7daa56

AdamOrmondroyd added 6 commits September 24, 2023 12:08

version bump

0b549bd

actually run correct test (should fail)

5d53524

use latest stable python

16f0ff0

put 3.x in the right place this time

8edb717

use --no-deps to avoid anesthetic overriding the latest versions

8a60ee8

remove cov

e5ec305

lukashergt mentioned this pull request Sep 27, 2023

Changes for pandas 2.1 #335

Merged

6 tasks

Merge branch 'master' into latesttest

e5032f3

increase python version in readme

b3434b1

lukashergt requested changes Oct 1, 2023

View reviewed changes

README.rst Outdated Show resolved Hide resolved

.github/workflows/CI.yaml Outdated Show resolved Hide resolved

AdamOrmondroyd added 6 commits October 1, 2023 22:23

3.9 duh

5b25b87

break up min/latest dependencies tests for output readability

8c1e854

Merge remote-tracking branch 'handley-lab/master' into latesttest

f4bdb46

version bump

5db5ae9

first pass at checking anesthetic is up to date (plus merge messed up…

80289c9

… pyproject.toml)

lets try a different number of brackets

9306b31

lukashergt reviewed Oct 1, 2023

View reviewed changes

.github/workflows/CI.yaml Outdated Show resolved Hide resolved

AdamOrmondroyd added 5 commits October 2, 2023 00:23

separate into two tests + double quotes

636bcbf

deliberately reduce pandas version

c59b5c1

Revert "deliberately reduce pandas version"

1aae3dc

This reverts commit c59b5c1.

tidy condition

0eef161

Revert "tidy condition"

8cf9914

This reverts commit 0eef161.

AdamOrmondroyd added 5 commits October 2, 2023 23:56

create bin/check_up_to_date.py which compares package version to pypi

77c3de9

remember to add new file

f245a9a

remember to install requests

f1aefb3

artificially restrict matplotlib

00c67dd

Revert "artificially restrict matplotlib"

f1d796f

This reverts commit 00c67dd.

AdamOrmondroyd added 4 commits October 3, 2023 00:24

use latest_dependencies to get list of packages (some gymnastics invo…

c11fbc1

…lved)

I guess I need tomli again

83e00dd

artificially restrict numpy

516683e

Revert "artificially restrict numpy"

2c52c6e

This reverts commit 516683e.

lukashergt previously approved these changes Oct 4, 2023

View reviewed changes

remove codecov

bec343f

AdamOrmondroyd dismissed lukashergt’s stale review via bec343f October 4, 2023 19:06

AdamOrmondroyd marked this pull request as ready for review October 4, 2023 19:06

lukashergt reviewed Oct 4, 2023

View reviewed changes

AdamOrmondroyd added 2 commits October 4, 2023 21:15

remove cov from minimum-dependencies

4cefb0d

specify that tests are in

33ceb6a

lukashergt approved these changes Oct 4, 2023

View reviewed changes

AdamOrmondroyd merged commit 2c01d13 into handley-lab:master Oct 4, 2023
22 checks passed

		- name: Upload coverage to Codecov
		uses: codecov/codecov-action@v1

Test latest dependencies #344

Test latest dependencies #344

Conversation

AdamOrmondroyd commented Sep 24, 2023 • edited Loading

Description

Checklist:

codecov bot commented Sep 24, 2023 • edited Loading

Codecov Report

AdamOrmondroyd commented Sep 24, 2023

lukashergt commented Sep 27, 2023

AdamOrmondroyd commented Oct 1, 2023

AdamOrmondroyd commented Oct 1, 2023

lukashergt left a comment

Choose a reason for hiding this comment

AdamOrmondroyd commented Oct 1, 2023

lukashergt commented Oct 2, 2023

AdamOrmondroyd commented Oct 2, 2023

AdamOrmondroyd commented Oct 2, 2023

AdamOrmondroyd commented Oct 2, 2023

lukashergt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdamOrmondroyd commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukashergt Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukashergt left a comment

Choose a reason for hiding this comment

AdamOrmondroyd commented Oct 4, 2023

lukashergt commented Oct 4, 2023

AdamOrmondroyd commented Oct 4, 2023

lukashergt commented Oct 4, 2023

AdamOrmondroyd commented Sep 24, 2023 •

edited

Loading

codecov bot commented Sep 24, 2023 •

edited

Loading

AdamOrmondroyd commented Oct 4, 2023 •

edited

Loading

lukashergt Oct 4, 2023 •

edited

Loading