test: test minimum stated dependencies on CI with uv --resolution lowest-direct
#109
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 76.41% 76.81% +0.40%
==========================================
Files 49 49
Lines 4956 4930 -26
==========================================
Hits 3787 3787
+ Misses 1169 1143 -26 ☔ View full report in Codecov by Sentry. |
|
woohoo! all green checks without re-running! :) |
|
sorry to bug @gselzer, would love to have our tests green again :) this isn't as big of a change as it looks from the line count. the biggest change for you is that it moves dev dependencies into |
There was a problem hiding this comment.
sorry to bug @gselzer, would love to have our tests green again :) this isn't as big of a change as it looks from the line count.
D'oh! Sorry I missed this.
You're utilizing many tools that I am not terribly familiar with, so I have no real suggestions for you, however I think this is a great step forward, and I'm excited to gain more exposure to uv!
One small concern that I have, looking back at the tests, is that ndv is hogging public (read: GitHub) resources. We have 40+ tests, each averaging a minute or more. Since that isn't really an effect of these changes (though re-running the tests will exacerbate the problem), that shouldn't hold up merging. Maybe I'll file a few issues.
| @@ -387,7 +387,7 @@ def _generate_clim_positions(self, npoints: int = 256) -> np.ndarray: | |||
| Y[2:-2] = np.linspace(0, 1, npoints) ** self._gamma | |||
| np.array([(np.mean(clims), 2**-self._gamma)]) | |||
|
|
|||
| return np.vstack((X, Y, Z), dtype=np.float32).transpose() | |||
| return np.vstack((X, Y, Z)).astype(np.float32).transpose() | |||
There was a problem hiding this comment.
Just curious why you made this change - did it fix tests, or help performance?
There was a problem hiding this comment.
fixes the minimum stated numpy version
There was a problem hiding this comment.
Aha! So it is...thanks for enlightening me!
...although, I think it's funny that you bumped NumPy from 1.22 to 1.23 in this PR, and 1.24 is the version introducing np.vstack's dtype parameter 😆
are you concerned for github on the whole? :) resources are restricted on an organization basis, so they don't let you use more than a certain amount at a time. that's why they don't all run immediately (you'll see them queue). can you clarify the key concern here? edit: I can definitely merge different parts of the matrix into a single job, but then our tests will take longer. |
"Key concern" sounds strong 😅 Ideally, we'd uncover the issues causing our segfaults and remove the repeated runs. That's the main thing I wouldn't mind having an issue tracking.
Well, I wouldn't change anything here. I don't want to merge jobs either, because flat > nested! |
ah... ok, well the good news here is that most of them aren't repeated... For example, most look like this 
(i.e. it only repeats if it fails, preventing me from having to manually go in and press "repeat failed tests"). occasionally we see this, where only the third test actually passed (attempt 3 must pass or it fails the test) 
I think having this in will help me repeat this locally much easier, because I can quickly test with various environments with a single make command |
|
ok thanks for having a look. Apologies if it causes any difficulty in local setup, and we can always add back a |
Absolutely! I'd just hope that we will eventually solve the segfaults instead of "hiding" them behind repeated runs.
Should be fine, I think I'll survive! |
don't worry, that's absolutely the goal. the problem is mostly pyside, and it almost entirely happens on CI, so it's A) not affecting users much and B) hard to debug. This just lets us at least keep moving on other stuff as long as it's not fundamentally all broken |
This PR is trying to make our tests a little more robust. I think the ultimate underlying cause has to do with retaining strong references to various qt objects, which is preventing proper cleanup. But the goal here isn't immediately to fix that. Rather, this establishes patterns for using
uvto quickly test many different environment configurations locally (and also uses that same setup on CI). There is a guide in CONTRIBUTING.md that should explain most of it.The key takeaway is that one can easily get setup here just running
uv sync. And then you can test any combo of extras withmake test extras=vispy,pyqt [min=1] [isolated=1] [py=3.10](brackets are optional features)The massive 6000+ line change here is due to the
uv.lockfile, which will be checked into source (makes it very easy to get reproducible dev environments withuv sync). I've also gone through and re-evaluated all of our minimum dependency pins, and test those as well on CI.For now, I've added a few "retries", that should minimize test failures due to segfaults. but i hope to remove these in the future after digging into the segfaults.
@gselzer, have a quick look and let me know if any of this would cramp your style. I know
makeisn't a builtin command on windows, but i believe it can be installed (alternatively we can usejust... but that also needs to be installed)