Conversation
Add 2 test on the file climada/entity/exposure/base.py under test.base.py. Both tests check the __init__ function of base.py. 1) The first test check that a specifc value is raised if the clomun name of geometry of a gdf is customized. 2) The second test checks the remaining generic attributes from derived classes (Made by Lukas).
Remove a test that was not finished in climada_python/entity/Exposures/test/test_base.py.
Removing a 2 lines from exposures/base.py
Fix formatting and test docstrings Co-authored-by: Emanuel Schmid <[email protected]>
Fix bug in test
| def test__init__mda_in_kwargs(self): | ||
| """Check if mda is not present in kwargs after being removed""" | ||
| litpop = LitPop(exponents=2) | ||
| self.assertEqual(litpop.exponents, 2) | ||
| litpop = LitPop(meta=dict(exponents=3)) | ||
| self.assertEqual(litpop.exponents, 3) |
There was a problem hiding this comment.
much better now 😉 though I still cannot make sense out of the test description. What is "mda", where is it removed and how is that related to the the exponents?
There was a problem hiding this comment.
This is due to the Exposures class being written quite badly, if I may say so. We try to test climada/entity/exposures/base.py#L178 and following. There is a class attribute _metadata which is supposed to be extended by derived classes. The base class __init__, however, is built to handle these extensions. L178 and L179 check if there are keywords in the current class type _metadata that is not part of the Exposures._metadata. This can only be the case if self is a derived class, and not Exposures itself. The following code will then extract these keywords from the kwargs before the remainder of the kwargs is passed to the GeoDataframe.
See Exposures._metadata and LitPop._metadata
This is all very convoluted, and the only advantage that I see is that you do not need a dedicated __init__ for derived classes anymore, at the expense of code that is very difficult to understand.
There was a problem hiding this comment.
Oh, what's even worse is that the Exposures.__init__ will thus instantiate attributes with names listed in _metadata, even for derived classes.
There was a problem hiding this comment.
I see. Then the suggested description would about describe the purpose of this test?
Co-authored-by: Emanuel Schmid <[email protected]>
3 participants
Changes proposed in this PR:
This PR add:
The first test, test line 497-498 of climada_python/entity/exposures/base.py. It check that if a geometry column is already present in the gdf, a specifc error message is displayed.
The second test, test lines 179-185 of climada_python/entity/exposures/base.py. It checks that if metadata is in kwargs it will be removed from it, and if it's present in meta, it will be added to the dictionary meta. Done by Lukas.
Remove a half finished and wrong test that was pushed in the first commit.
Remove 2 lines of Exposures/base.py that are no longer useful
PR Author Checklist
develop)PR Reviewer Checklist