home / github / issue_comments

Menu
  • GraphQL API
  • Search all tables

issue_comments: 898142259

This data as json

html_url issue_url id node_id user created_at updated_at author_association body reactions performed_via_github_app issue
https://github.com/pydata/xarray/pull/5690#issuecomment-898142259 https://api.github.com/repos/pydata/xarray/issues/5690 898142259 IC_kwDOAMm_X841iJAz 5635139 2021-08-13T02:51:59Z 2021-08-13T02:51:59Z MEMBER

I would suggest that new tests use -> None: so mypy checks them, and we build up some tests for our typing through that — any thoughts?

I'm not opposed to suggesting -> None for tests, but I would hesitate to require it:

* It's not unusual for tests to be incompatible with type checking, e.g., because they are checking invalid input, or rely on tricks like mocks.

* It's also a fair amount of boilerplate to add to code that already has a lot of boilerplate (for things like parametrized tests & fixtures), so readability could suffer.

Suggesting is fine.

I might vote to strongly suggest — it does worry me that a bunch of our type annotations are wrong and we don't have a mechanism to assess that.

But for sure if someone is using mocks etc, then no need. And I'm not suggesting everything (e.g. fixtures) needs to be annotated — adding -> None: will let mypy test the types it already knows about.

{
    "total_count": 0,
    "+1": 0,
    "-1": 0,
    "laugh": 0,
    "hooray": 0,
    "confused": 0,
    "heart": 0,
    "rocket": 0,
    "eyes": 0
}
  965939431
Powered by Datasette · Queries took 3.084ms · About: xarray-datasette