Support file objects (streams, sockets etc.) #909
Conversation
|
📖 Docs for this PR can be previewed here |
benjeffery
force-pushed
the
file-objects
branch
from
3930036 to
c3b2d6c
Compare
Codecov Report
@@ Coverage Diff @@
## main #909 +/- ##
==========================================
- Coverage 93.52% 93.47% -0.05%
==========================================
Files 25 25
Lines 19963 20029 +66
Branches 790 796 +6
==========================================
+ Hits 18671 18723 +52
- Misses 1259 1272 +13
- Partials 33 34 +1
Continue to review full report at Codecov.
|
|
There seems to be an issue with codecov not showing files, so haven't checked coverage yet. Will look into it. |
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
LGTM! I haven't pored over the details of the tricky stuff, but I assume this is derived from what we did in kastore, so I don't need to?
| # SOFTWARE. | ||
| """ | ||
| Configuration and fixtures for pytest. Only put test-suite wide fixtures in here. Module | ||
| specific fixtures should live in their modules. |
There was a problem hiding this comment.
This is great! Can you just put in either a quick example of how to use these fixtures, or perhaps a link to the documentation from pytest for how this this is done?
Yep, a lot of this was copied over. |
python/tests/conftest.py
Outdated
|
|
||
| # A tree sequence with data in all fields | ||
| @fixture(scope="session") | ||
| def ts(): |
There was a problem hiding this comment.
This is great, but I worry a bit about attaching meaning to the (very commonly used) variable ts here. I can see this being confusing for people. What if we adopt a convention like having a suffix or prefix of fixture for these parameters? That might make things a bit more obvious.
There was a problem hiding this comment.
Yeah, this kind of magic can be really confusing if you aren't aware what it going on. It seemed a bit much to put a suffix on, but when I think about the reader of the code it really does help. I've added them in b0b566b
There was a problem hiding this comment.
I agree, it feels a bit wordy in the code, but I think it'll help people who aren't used to using pytest.
python/tests/conftest.py
Outdated
| @fixture(scope="session") | ||
| def multi_ts(): | ||
| return [msprime.simulate(i, random_seed=42) for i in range(2, 100, 5)] | ||
| def multi_ts(ts): |
There was a problem hiding this comment.
This is a bit vague, as I'd imagine we'll want all sorts of different fixtures like this. What about replicate_ts_fixture which will return a list of msprime replicate sims?
|
LGTM, merge away whenever! |
benjeffery
force-pushed
the
file-objects
branch
from
b0b566b to
1960d7c
Compare
AdminBot-tskit
force-pushed
the
file-objects
branch
from
1960d7c to
95c1798
Compare
3 participants
Closes #657
WIP! Using PR for testing on WIN/OSX.