LiamPattinson commented 2 years ago

I've updated FusionPointSource and FusionRingSource to store their inputs as properties, which allows them to be checked for validity. Info on DT/DD fuel types has been moved to a new class, which may be refactored later if work on Issue https://github.com/fusion-energy/openmc-plasma-source/issues/37 goes ahead. I've also added unit tests for these new features.

codecov[bot] commented 2 years ago

Codecov Report

Merging #51 (4c80c3d) into develop (e430db5) will increase coverage by 2.25%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop      #51      +/-   ##
===========================================
+ Coverage    94.50%   96.75%   +2.25%     
===========================================
  Files            6        7       +1     
  Lines          200      216      +16     
===========================================
+ Hits           189      209      +20     
+ Misses          11        7       -4

Impacted Files	Coverage Δ
openmc_plasma_source/fuel_types.py	`100.00% <100.00%> (ø)`
openmc_plasma_source/point_source.py	`100.00% <100.00%> (+13.33%)`	:arrow_up:
openmc_plasma_source/ring_source.py	`100.00% <100.00%> (+11.11%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e430db5...4c80c3d. Read the comment docs.

shimwell commented 2 years ago

Thanks @LiamPattinson this PR looks great to me, I like the separation of the fuel part and the reuse of this class in both the ring source and point source. As you mention this will help if #37 goes ahead.

I think we can merge this unless there are any objections @RemDelaporteMathurin

I have a slight related question on proper_tea as a dependency.

fusion-energy / openmc-plasma-source

Improved point and ring sources #51

Codecov Report