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

Pdep fixes #60

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Pdep fixes #60

wants to merge 4 commits into from

Conversation

alongd
Copy link
Member

@alongd alongd commented Aug 24, 2020

Three bug fixes:

  • In schema check_pdep_only_if_gas_phase(), reactor is an object
  • T and P increments in RMG's PDep block must be integers
  • Check the PDep method before using the METHOD_MAP dict

Please don't merge yet, I should add tests

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2020

Codecov Report

Merging #60 into master will decrease coverage by 4.86%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #60      +/-   ##
==========================================
- Coverage   74.98%   70.11%   -4.87%     
==========================================
  Files           9        9              
  Lines        1491     1499       +8     
  Branches      381      384       +3     
==========================================
- Hits         1118     1051      -67     
- Misses        238      323      +85     
+ Partials      135      125      -10     
Impacted Files Coverage Δ
t3/schema.py 73.84% <22.22%> (-0.83%) ⬇️
t3/utils/writer.py 70.81% <33.33%> (-15.06%) ⬇️
t3/main.py 67.71% <0.00%> (-5.96%) ⬇️
t3/logger.py 62.30% <0.00%> (-2.31%) ⬇️
t3/common.py 76.47% <0.00%> (-1.97%) ⬇️

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 f44de87...2382664. Read the comment docs.

@alongd alongd force-pushed the pdep_fixes branch 3 times, most recently from c7389b3 to 77c68bf Compare August 26, 2020 02:37
not a dict.
Note that it is a dict under `check_species_and_reactors()` which is a root_validator, on line 503. Cofusing, but crashes otherwise
The first iteration will assign the value from this dictionary to pdep['method', the T3 will crash at the second iteration, since the corresponding value is not a key in this dict. Here we first check whether the value is already correct, if not we use the mapping dict.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants