Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Broken Latex Code #1891

Merged
merged 4 commits into from Feb 16, 2022
Merged

Fixing Broken Latex Code #1891

merged 4 commits into from Feb 16, 2022

Conversation

bartnikm
Copy link
Contributor

@bartnikm bartnikm commented Feb 8, 2022

Description

Places were latex code did not display correctly were updated to allow the code to display correctly.

Motivation and context

There were some places where code written in latex was formatted or written incorrectly

How has this been tested?

  • Testing pipeline.
  • Other.

Examples

https://bartnikm.github.io/tardis-bartnikm/branch/latex-names/physics/setup/plasma/helium_nlte.html

Type of change

  • Bug fix.
  • New feature.
  • Breaking change.
  • None of the above.

Checklist

  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
    • (optional) I have built the documentation on my fork following the instructions.
  • I have assigned and requested two reviewers for this pull request.

@codecov
Copy link

codecov bot commented Feb 8, 2022

Codecov Report

Merging #1891 (42aa8d9) into master (c705946) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 42aa8d9 differs from pull request most recent head 4758856. Consider uploading reports for the commit 4758856 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1891   +/-   ##
=======================================
  Coverage   62.55%   62.55%           
=======================================
  Files          68       68           
  Lines        7127     7128    +1     
=======================================
+ Hits         4458     4459    +1     
  Misses       2669     2669           
Impacted Files Coverage Δ
tardis/tardis/plasma/properties/atomic.py 61.98% <0.00%> (+0.22%) ⬆️

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 c705946...4758856. Read the comment docs.

@tardis-bot
Copy link
Contributor

Before a pull request is accepted, it must meet the following criteria:

  • Is the necessary information provided?
  • Is this a duplicate PR?
    • If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
    • If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
  • Does it pass existing tests and are new tests provided if required?
    • The test coverage should not decrease, and for new features should be close to 100%.
  • Is the code tidy?
    • No unnecessary print lines or code comments.

Another `helium_treatment` option offered by TARDIS is `numerical-nlte`. The use of this plasma property requires an additional code that is the property of Stephan Hachinger (see arXiv:1201.1506) and is not distributed with TARDIS. TARDIS also requires a specific atomic datafile to use this module. This plasma option is included so that people who have access to and permission to use the necessary module may use it. Otherwise, the `recomb-NLTE` option provides a very accurate alternative approximation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What actually changed here? I can't tell from the changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not edit or look at this section, so I'm not sure what, if anything, was changed here either.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found that sometimes github just does that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normally github does not change things. What does your local diff say?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is potentially a different newline character due to this being edited via Jupyter Notebook on a Windows OS.

@isaacgsmith
Copy link
Member

I think we should wait on this one until #1868 has been merged. That way we can rebase and check that this does fix the issue in the plasma graph. But it looks good to me

@bartnikm bartnikm self-assigned this Feb 11, 2022
@wkerzendorf wkerzendorf merged commit fca5ab7 into tardis-sn:master Feb 16, 2022
@bartnikm bartnikm deleted the latex-names branch February 17, 2022 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants