search

cyclus / cymetric

http://fuelcycle.org/user/cymetric/index.html
Other
5 stars 20 forks source link

add units in inventory #154

Closed bam241 closed 5 years ago

bam241 commented 5 years ago

this should fix error in cyclus/cyclus#1492 which adds units to TimeSeries and inventory tables

gonuke commented 5 years ago

I just merged the Cyclus change - can you relaunch tests with the updated image?

gonuke commented 5 years ago

This PR does nothing but whitespace changes...????

bam241 commented 5 years ago

this is weird, let me check ! the test were failing so I fixed it, while fixing it, I removed the change :(

bam241 commented 5 years ago

I know what appended!! I got tricked, we rely on cycamore:latest container, only the Cyclus:latest got updated....

bam241 commented 5 years ago

@gonuke this should be in order now. (forced cycamore docker update, with an empty PR and put back my changes) Sorry about that.

gonuke commented 5 years ago

This adds units to inventories, but not timeseries... is that not tested?

bam241 commented 5 years ago

this is correct.

We don't have test for specific root metrics, as they should be natively translated into Panda DataFrame. So it is somehow tested through the test_root_metrics.py, but the translator can translate any table shape into a PDF. Shape is not hard coded...

gonuke commented 5 years ago

Thanks @bam241 - will you update the cymetric image used for upstream testing?

bam241 commented 5 years ago

this should be done automatically