-
-
Notifications
You must be signed in to change notification settings - Fork 44
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
add vald shortlist parsing as well as handling for wavelength in vacuum/air and nm/aa #386
add vald shortlist parsing as well as handling for wavelength in vacuum/air and nm/aa #386
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #386 +/- ##
==========================================
+ Coverage 68.54% 68.65% +0.11%
==========================================
Files 30 30
Lines 3608 3650 +42
==========================================
+ Hits 2473 2506 +33
- Misses 1135 1144 +9 ☔ View full report in Codecov by Sentry. |
|
||
self._vald_columns = ( | ||
self.vald_shortlist_columns.copy() | ||
if shortlist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of ternary operators, but I also won't argue strongly if you want to keep this one
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the notebook
carsus/conftest.py
Outdated
@@ -103,7 +103,7 @@ def memory_session(): | |||
|
|||
@pytest.fixture(scope="session") | |||
def data_dir(): | |||
return os.path.join(os.path.dirname(__file__), "tests", "data") | |||
return Path(__file__).parent / "tests" / "data" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not make this a fixture but a single constant DATA_DIR_PATH
|
||
# Need to identify the wavelength column header and overwrite the wavelength to obtain units and air or vacuum | ||
# Also need to identify if Vmic is in the columns for correct column construction | ||
for line in content.split("\n")[:10]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make an issue about pattern matching.
Created issue #389 as a future project idea. |
📝 Description
Type: : 🚀
feature
|This PR modifies the vald parser to add some extra functionality for more varied vald outputs.
There are some missed lines in codecov to account for if the units of the wavelengths are in nm instead of angstroms, but I can't cover them unless I upload more data files and I've been trying to be sparing with that. I can upload them and cover them if that'd be preferred.
🚦 Testing
☑️ Checklist
build_docs
label