Implemented Model Reader for CMFGen example (Also refactored Stella Reader) #2349
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
xansh
force-pushed
the
stella-cmfgen-reader
branch
from
ced4d01 to
d4b4cd8
Compare
tardis/io/model/stella.py
Outdated
| raise ValueError( | ||
| '"mass of cell" is required in the Stella input file to infer columns' | ||
| ) | ||
| elif i + 1 == DATA_START_ROW: |
There was a problem hiding this comment.
Why are you checking i+1 in a loop? Won't the loop reach i+1 in the next iteration anyway?
tardis/io/model/stella.py
Outdated
| @@ -74,7 +75,7 @@ def read_stella_model(fname): | |||
| data = pd.read_csv( | |||
| fname, | |||
| delim_whitespace=True, | |||
| skiprows=DATA_START_ROW, | |||
| skiprows=DATA_START_ROW + 1, | |||
There was a problem hiding this comment.
Does this mean that DATA_START_ROW should start with +1 to begin with?
tardis/io/model/cmfgen.py
Outdated
| header_re_match = header_re[i].match(line) | ||
|
|
||
| metadata[HEADER_RE_STR[i][1]] = header_re_match.group(1) | ||
| elif i + 1 == DATA_START_ROW: |
There was a problem hiding this comment.
See comment on Stella parser
tardis/io/model/cmfgen.py
Outdated
| data = pd.read_csv( | ||
| fname, | ||
| delim_whitespace=True, | ||
| skiprows=DATA_START_ROW + 1, |
There was a problem hiding this comment.
See comment on Stella parser
There was a problem hiding this comment.
I'm afraid, looking at the code, I think this has to be this way, or somewhere there has to be a +1 or -1. But I've eliminated this from other places. Perhaps because there are empty lines between sections.
|
@wkerzendorf this should be reviewed and merged alongside the work you have been doing I think |
|
@atharva-2001 needs the stella changes removed and comments addressed (ideally). Tests would also be great. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2349 +/- ##
==========================================
- Coverage 68.73% 68.18% -0.56%
==========================================
Files 165 168 +3
Lines 14001 14216 +215
==========================================
+ Hits 9624 9693 +69
- Misses 4377 4523 +146 ☔ View full report in Codecov by Sentry. |
atharva-2001
force-pushed
the
stella-cmfgen-reader
branch
2 times, most recently
from
320858b to
fdb51b8
Compare
📝 Description
Type: 🚀
featureCreated a simple Model reader for CFMGen files, which segregates the input into data and metadata. Also refactored Stella Reader from #2327 .
🚦 Testing
How did you test these changes?
☑️ Checklist
build_docslabel