New metrics to return Transaction table with quantity in units of kg/GWe #161
Conversation
…antity_per_gwe function
louishartono
changed the title
There was a problem hiding this comment.
Thx @louishartono for adding this, this look great so far!
Some syntax changes are nevertheless necessary, we follow PEP8 suggestion about code style. Please try to modify your code accordingly.
Mainly:
- no space after
, - lines should be shorter than 80 chars
- no tab, for indentation use multiple of 4 spaces
- argument of a method should be aligned (we split in multiple lines)
cymetric/metrics.py
Outdated
| 'Commodity': tranacts.Commodity, | ||
| 'Units': tranacts.Units, | ||
| 'Quantity': tranacts.Quantity}, | ||
| columns=['SimId','TransactionId','ResourceId','ObjId','Time','SenderId','ReceiverId','Commodity','Units','Quantity']) |
There was a problem hiding this comment.
same here, I feel like you are redeclaring a copy of transacts, why is that required ?
There was a problem hiding this comment.
I tried deleting it but it caused an error because the name of the time column is 'TimeCreated' in tranacts while it is 'Time' in power.
There was a problem hiding this comment.
so maybe remake one of the column, instead of making a copy ?
tests/test_metrics.py
Outdated
| exp = pd.DataFrame(np.array([ | ||
| (UUID('f22f2281-2464-420a-8325-37320fd418f8'), 1, 7, 3, 3, 10, 20, 'LWR Fuel', 'kg/GWe', 0.82), | ||
| (UUID('f22f2281-2464-420a-8325-37320fd418f8'), 2, 8, 4, 3, 20, 30, 'FR Fuel', 'kg/GWe', 0.61), | ||
| (UUID('f22f2281-2464-420a-8325-37320fd418f8'), 3, 9, 5, 12, 30, 40, 'Spent Fuel', 'kg/GWe', 0.09), |
There was a problem hiding this comment.
Wouldn't it be confusing if I separate this line?
There was a problem hiding this comment.
no, PEP8 recommandation are line should be shorter than 80 chars to improve readability,
I might seem unintuitive, but this way readers don't have to scroll horizontally
|
This is good, we are almost there ! |
Made some changes to metrics.py. Added a new metrics called "TransactionQuantityPerGWe" to return the transactions table with quantity in units of kg/GWe instead of kg and created some tests to check the expected output as well. Feel free to give feedback or suggestions :).