Increase test coverage for GeometryCollection class#951
Increase test coverage for GeometryCollection class#951mwtoews merged 4 commits intoshapely:masterfrom
Conversation
|
@tomplex I'm in favor, but also want to check in with @jorisvandenbossche who is doing much of the 2.0 work. |
|
Yes, also +1 on this, both for better usage of pytest as for trying to get better test coverage. |
Yes please! The unittest way was fine 10 years ago, but I much prefer to see a modern pytest approach, as done in this PR. I have no idea if it really changes the coverage metrics (+0.1%??), but this is secondary to maintaining to modern conventions. |
Coverage increased because I added some test cases for things which weren't covered before: the Either way, since it seems this is popular I'll keep on trucking down this path with added focus on updating tests to use |
|
I am personally in favor of smaller PRs as well (maybe updating a few files combined in a single PR is fine, but I would certainly not do a single overhaul of all test files in one PR). |
|
Apart from the unrelated empty-collection discussion (opened a new issue for that -> #956), this PR is ready to be merged? |
|
Yes, I believe this is ready to go. |
mwtoews
left a comment
There was a problem hiding this comment.
Just two is_empty checks to add, otherwise looks good!
We're about to experience a good bit of churn with the push to Shapely 2.0 & merge with PyGEOS, so this seemed like a good time to work on increasing test coverage, especially in the geometry classes where I expect we will be trying to keep the interface and behavior identical while completely changing the internal workings. I also would like to consolidate tests to use Pytest, as it seems like most new tests have been written using that framework, and I think it'd be nice to have more consistency there.
If you're a 👍 on this, I'll keep adding to this or open another PR for more work, whatever is preferred. If not, no worries, but I wanted to check in to see what other folks thought before I did too much.