Support for assumption directive #73

[shailesh1729]

This patch provides support for the assumption directive. Code, as well as documentation, has been added for this directive. As described in issue #73, assumptions are frequently used and explicitly stated in the mathematical analysis of optimization algorithms.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[mmcky]

@AakashGfude can you please review this.

[AakashGfude]

Hi, @shailesh1729 @mmcky this looks good to me. We need to add a test and add a corresponding Assumption JSON file in translations. And then we should be good to merge.
I am not too fused about the color choice, but @mmcky let us know If you have any opinions.

Sample:

[AakashGfude]

Appreciate the work on this PR @shailesh1729, let me know if you need help in writing test etc. The tests are failing here because of a separate issue, which we can resolve later.

[mmcky]

Appreciate the work on this PR @shailesh1729, let me know if you need help in writing test etc. The tests are failing here because of a separate issue, which we can resolve later.

thanks @AakashGfude and @shailesh1729. Is there a way we could add colour as an option to the directive? @AakashGfude are there any standard color definitions in the theme?

[mmcky]

Appreciate the work on this PR @shailesh1729, let me know if you need help in writing test etc. The tests are failing here because of a separate issue, which we can resolve later.

@AakashGfude is this issue resolved on main? If yes then we should update this PR with the latest upstream. If no let's fix it on main so we can get tests passing on this branch.

[AakashGfude]

thanks @AakashGfude and @shailesh1729. Is there a way we could add colour as an option to the directive? @AakashGfude are there any standard color definitions in the theme?

@mmcky not at present. But we can add that as a feature in the future.
We have certain colors defined as variables:

  --note-title-color: rgba(68,138,255,.1);
  --note-border-color: #007bff;
  --warning-title-color: rgba(220,53,69,.1);
  --warning-border-color: #dc3545;
  --hint-title-color: rgba(255,193,7,.2);
  --hint-border-color: #ffc107;
  --caution-title-color: rgba(253,126,20,.1);
  --caution-border-color: #fd7e14;
  --grey-title-color: rgba(204,204,204,.2);
  --grey-border-color: #ccc;

which we keep using between the directives, but don't have any defined spec as such.

[codecov-commenter]

Codecov Report

Merging #80 (fffc6ac) into master (16ae061) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   96.08%   96.13%   +0.04%     
==========================================
  Files           6        6              
  Lines         358      362       +4     
==========================================
+ Hits          344      348       +4     
  Misses         14       14              

Flag	Coverage Δ
pytests	96.13% <100.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
sphinx_proof/nodes.py	93.51% <100.00%> (+0.12%)	⬆️
sphinx_proof/proof_type.py	100.00% <100.00%> (ø)

---

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 16ae061...fffc6ac. Read the comment docs.

[AakashGfude]

@mmcky have merged master to this. And I think this is good to go apart from deciding on the color scheme. We can add translations to this in a later PR.

[shailesh1729]

thanks @AakashGfude and @shailesh1729. Is there a way we could add colour as an option to the directive? @AakashGfude are there any standard color definitions in the theme?

Thanks for your review. I used one of the colors from one of the existing directives. I didn't introduce any new colors to the mix.

[shailesh1729]

@AakashGfude, thanks for your kind review. I have added an Assumption.json file in translations/jsons directory. However, I am not sure how I need to add a test for the same. It looks like the test_convert function in test_locale_convert automatically adds test for each file.

I couldn't figure out the correct translation of "assumption" in other languages. Google translate seemed to suggest words based on hypothesis or supposition in French/Spanish. So I have kept the English one only in the JSON file.

[shailesh1729]

Is there a plan to support a generic numbered directive where the actual name and color of the directive can be user controlled? In LateX one can create new theorem environments with whatever name one chooses. I keep thinking of a few more directive types like "fact", "hypothesis", "exercise" "challenge", etc.. Some directives are too specific to a particular domain [e.g. "constraint qualification" which is specific for Lagrangian duality].

[AakashGfude]

Is there a plan to support a generic numbered directive where the actual name and color of the directive can be user controlled? In LateX one can create new theorem environments with whatever name one chooses. I keep thinking of a few more directive types like "fact", "hypothesis", "exercise" "challenge", etc. Some directives are too specific to a particular domain [e.g. "constraint qualification" which is specific for Lagrangian duality].

@shailesh1729 This can be a good feature. Thanks for bringing this up. I think we should create an issue regarding this in this repo. Would you like to create one? Then we can plan it in one of our development cycles.

[AakashGfude]

Thanks, heaps @shailesh1729 for this PR. I think it's good to merge as is. We can discuss color schemes etc in issues and later PRs.

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️

[shailesh1729]

@shailesh1729 This can be a good feature. Thanks for bringing this up. I think we should create an issue regarding this in this repo. Would you like to create one? Then we can plan it in one of our development cycles.

Created #84

Thank you for merging this PR.