Make PACs references to Commoditys

[alexdewar]

Description

Now that we have merged the commodity-related code, we can use these data structures elsewhere.

As PACs are commodities, it makes sense to represent them as references to commodity types, which is what I've done here. There are currently no tests, but we can add these later.

Edit: PACs are "Primary Activity Commodities": From the glossary:

"Primary Activity Commodity (PAC): The PACs specify which output/s are linked to the process
capacity. The combined output of all PACs cannot exceed the Asset’s capacity. A user can define
which output/s are PACs."

Closes #164.

Type of change

• Bug fix (non-breaking change to fix an issue)
• New feature (non-breaking change to add functionality)
• Refactoring (non-breaking, non-functional change to improve maintainability)
• Optimization (non-breaking change to speed up the code)
• Breaking change (whatever its nature)
• Documentation (improve or add documentation)

Key checklist

• All tests pass: $ cargo test
• The documentation builds and looks OK: $ cargo doc

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[codecov]

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.04%. Comparing base (4e88827) to head (82fdd78).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/model.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #154      +/-   ##
==========================================
+ Coverage   84.69%   85.04%   +0.35%     
==========================================
  Files          12       12              
  Lines         392      408      +16     
==========================================
+ Hits          332      347      +15     
- Misses         60       61       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dalonsoa]

In principle this looks ok - it follows a similar structure that other input files. But:

• it's unclear to me what PACs are
• none of the functions implemented have any doscstring. It costs much less effort to write the documentation as you code along
• I don't see any reason for not having tests now

Let's try to get into the habit of producing code that is properly documented and tested and not rush things through.

[alexdewar]

Ok. This one is admittedly a bit rough around the edges... I've got a couple of other branches that I think are ready to go, so I'll open those as PRs when I get a chance then circle back to this. I'll mark it as draft for now.

[alexdewar]

Following @dalonsoa's feedback, I've added tests and doc comments, so I think this is now ready for (re-)review.

I've tagged everybody who's involved in the project, but don't feel any obligation.

[dalonsoa]

I've a couple of questions/uncertainties due to my lack of familiarity, for now, with rust. Keeping those aside, this looks good!

[dalonsoa]

This is just to convert an iterator into a list (well, vector), right?

[alexdewar]

Yep.

[dalonsoa]

Isn't it a bit odd to make it a list just to make it an iterator again? Cannot we use into_iter all the time to get references to the elements within and re-use them rather than re-creating the iterator?

I've found this comment quite enlightening - I think. Unless I got it all wrong. https://stackoverflow.com/a/34745885/3778792

[alexdewar]

Unfortunately you can't reuse iterators once you've iterated over them once (like in Python, I think).

I didn't like that we were making an unnecessary Vec and iterating over it twice though, so I've reworked it to perform the duplicate check in the lambda we pass to map().

[dalonsoa]

So, why using iterators at all (in this case)? Let's just use a list/vector from the onset and iterate over it again and again... or I'm getting this wrong?

[alexdewar]

That would be another way to write the interfaces for these functions (actually, that's what I did originally). Now we just pass in an iterator which yields CSV data one row at a time instead.

I've come to the conclusion that using iterators is often a nicer way to do things in Rust though. For one thing, it means you don't have to make a vector just for the sake of passing it into a function, then throwing it away again afterwards, which would mean lots of pointless copying of data. (But if you do happen to have data in a Vec, you can pass that in too!) It's also much more flexible, because you can pass in the contents of a vector, the values in a HashMap, the contents of a CSV file etc. Plus, you can chain operations with iterators (e.g. map, filter etc.) then pass the result somewhere else, again without having to save any of this into data structures anywhere until the point where you actually want to store it for some reason.

[AdrianDAlessandro]

I have a couple of basic questions. I'm not following the syntax in new code that has been added, so I'll need to go through it slowly, or have it explained to me. But wanted to get these questions in first.

[AdrianDAlessandro]

Thanks for taking me through this!

[AdrianDAlessandro]

Just missing and entry for iter in the docstring