Add inspector for supercapacitor. #242
Conversation
|
Somewhere in your test please pass an equivalent circuit to the inspector and make sure in throws the appropriate exception. |
Current coverage is 93.42% (diff: 95.83%)@@ master #242 diff @@
==========================================
Files 72 72
Lines 4999 5017 +18
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 4670 4687 +17
- Misses 329 330 +1
Partials 0 0
|
Current coverage is 93.42% (diff: 96.29%)@@ master #242 diff @@
==========================================
Files 72 72
Lines 4999 5020 +21
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 4670 4690 +20
- Misses 329 330 +1
Partials 0 0
|
Current coverage is 93.64% (diff: 100%)@@ master #242 diff @@
==========================================
Files 72 72
Lines 4999 5051 +52
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 4670 4730 +60
+ Misses 329 321 -8
Partials 0 0
|
Rombur
force-pushed
the
inspector
branch
2 times, most recently
from
33c0188
to
720e48f
Compare
Rombur
changed the title
|
OK this is ready for review. |
|
You should check that the docker compose works for you. |
| if (supercapacitor == nullptr) | ||
| { | ||
| std::bad_cast exception; | ||
| throw exception; |
There was a problem hiding this comment.
throw std::bad_cast();
Please comment and explain why you had to throw the exception yourself.
| geometry->get_triangulation(); | ||
| write_mesh("output_test_geometry_2.vtu", geometry->get_triangulation()); | ||
| const unsigned int n_cells = 1536; | ||
| BOOST_CHECK(n_cells == triangulation->n_active_cells()); |
There was a problem hiding this comment.
Would you annotate to say that the test just constructs the geom and that you just check by hand the number of cells?
| } | ||
| else | ||
| throw std::runtime_error("Unknown inspector type"); | ||
|
|
There was a problem hiding this comment.
Would you use a loop here
for inspector : {2d ,3d}
try
device.inspect
break
catch bad_cast
if caught bad_cast each time throw a runtime exception
| @@ -4,6 +4,9 @@ | |||
| namespace pycap | |||
| { | |||
|
|
|||
| // Macro to enable default arguments | |||
| BOOST_PYTHON_FUNCTION_OVERLOADS(inspect_overloads, inspect, 1, 2) | |||
There was a problem hiding this comment.
Is it possible to document what is the default value for the string argument?
| device = EnergyStorageDevice(ptree) | ||
| # data = device.inspect('postprocessor') | ||
| # self.assertTrue(isinstance(data, dict)) | ||
| # self.assertEqual(len(data), 0) |
|
Ready for re-review. |
| device = EnergyStorageDevice(ptree) | ||
| data = device.inspect('postprocessor') | ||
| self.assertTrue(isinstance(data, dict)) | ||
| self.assertEqual(len(data), 0) |
There was a problem hiding this comment.
You don't want to check that a .vtu file has been created here?
There was a problem hiding this comment.
BTW, empty dictionaries evaluate to False in Python so you could do self.assert(not data).
There was a problem hiding this comment.
You may use os.path.isfile(path) to check that the file has been created.
| inspectors.push_back(std::shared_ptr<cap::EnergyStorageDeviceInspector> ( | ||
| new cap::SuperCapacitorInspector<2>())); | ||
| inspectors.push_back(std::shared_ptr<cap::EnergyStorageDeviceInspector> ( | ||
| new cap::SuperCapacitorInspector<3>())); |
There was a problem hiding this comment.
This is a good place for std::make_shared but you don't need to change it.
| @@ -59,6 +59,20 @@ def test_inspect_device(self): | |||
| self.assertTrue(key in data) | |||
| print(data) | |||
|
|
|||
| def test_postprocessor_inspect(self): | |||
There was a problem hiding this comment.
I just realized we forgot to do self.assertRaises(RuntimeError, device.inspect, 'invalid')
|
Done |
I have a problem with matplotlib so I can't run any tests... I will use the PR to run the tests, so don't merge this.