Codecov Report

Merging #61 (31a340d) into master (834549e) will increase coverage by 0.0%.
The diff coverage is 99.2%.

@@           Coverage Diff           @@
##           master     #61    +/-   ##
=======================================
  Coverage    98.7%   98.7%            
=======================================
  Files         117     118     +1     
  Lines        2179    2319   +140     
  Branches      504     556    +52     
=======================================
+ Hits         2151    2290   +139     
  Misses         12      12            
- Partials       16      17     +1     

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 834549e...31a340d. Read the comment docs.

Hi @georgios-ts. Great effort here, I think this is looking really great so far, so that's fantastic :)

Regarding your question:

Is toqito/helper the best place for npa_hierarchy.py or we should expose the functionality of npa_constraints to the user?

Hmm, that is a good question. Some candidate ideas are to place it directly within the NonlocalGame class. Since the NPA hierarchy really only applies to functionality pertaining to a nonlocal game anyway, it might make sense to consolidate it there. The downside of that of course is that it bloats the code. I think for now though, where you have it makes sense to me at the moment.

In short, I'm not too strongly opinionated as to where it goes. You obviously are a very capable and experienced developer, and part of the (selfish) benefit from my perspective is that this gives me an opportunity to learn from you. This is perhaps a broader point, but if you have feedback on a larger scale about the structure of the project, I would love to hear that from you. Anyway, that's all tangential--the takeaway there is I will let you use your best discretion on that front.

Regarding the second part of your question, I think you did a great job in exposing the proper interface to the user--namely, the function you wrote inside the NonlocalGame class is probably all we wish the user to have exposure to.

As I said, overall, looks great. I added quite a few comments in the diff, but rest assured, they are all quite minor.

Do let me know if you have any questions, disagree about my comments, or have any input at all. Cheers!

Thanks @vprusso.
Ok, my suggestion is to leave it where it is for now (in toqito/helper).

Honestly, the structure of the project is great! It's simple, clean, easy to navigate and each function is well documented.

Hi @georgios-ts. Great job addressing all of my comments. I know a lot of them were quite pedantic and trivial, but thank you kindly for taking the time to alter them. I see you made some updates and I just wanted to be sure that you aren't waiting for any input on my end. Of course, if you are still chugging along, I also don't want to be overly burdensome. Basically just checking in to make sure you're not waiting on me :)

Hi @vprusso, this is ready from my side.

@georgios-ts. Fantastic, I'll merge away! :)