metno / MetAlert-Search

Polygon search and indexing package for Annotated Atlas
Apache License 2.0
1 stars 0 forks source link

Dev/4 polygon wrapper class #14

Closed fzeiser closed 3 years ago

fzeiser commented 3 years ago

Summary: Potentially this should be rebased on the lastest version of #13 instead. [Edit: Rebased Fri 25 June, 17:06 to get test coverage for data class]

Related Issue: Resolves #4.

Suggested reviewer(s):

Reviewer checklist:

fzeiser commented 3 years ago

[Flake 8 not passing now, pretty sure it passed before. I can fix comments related to flake8 after you have had a look at the branch...]

fzeiser commented 3 years ago

I keept the monkeypatch part. Actually it works, it was just about how I write them. Why should I monkeypatch with pytest.monkeypatch instead of setting the methods directly: Because it can be used as a context, so in a with block, where it does not affect the rest of the test function. This can be handy, to make sure that one has not accidentally changed the object.

BTW: The issue that we had a look at this afternoon was of course a stupid one. I created a file and did not think about the fact that we currently use a persistent tmp directory (as opposed to the pytest default tmpdir factory with tmp_path, which creates a unique tmp folder for each test ( and keeps it for, eh, 3 times). Btw: Is there a reason not to use pytest.tmp_path?

vkbo commented 3 years ago

BTW: The issue that we had a look at this afternoon was of course a stupid one. I created a file and did not think about the fact that we currently use a persistent tmp directory (as opposed to the pytest default tmpdir factory with tmp_path, which creates a unique tmp folder for each test ( and keeps it for, eh, 3 times). Btw: Is there a reason not to use pytest.tmp_path?

Because I want to control the creation and deletion of temp folders. I usually also have a fixture for function temp folders, which I can add to this project. Being able to control the fixtures myself makes it significantly easier to debug tests.

The code to create and clean up such folders is trivially easy to write, so there isn't much benefit in using the built-in feature.