feature/ES-803 pressure to altitude

[eoneill-mide]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 98.27586% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.35%. Comparing base (a75ced3) to head (5e1bc05).
⚠️ Report is 15 commits behind head on development.

Files with missing lines	Patch %	Lines
endaq/calc/utils.py	98.27%	1 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff               @@
##           development     #229      +/-   ##
===============================================
+ Coverage        70.94%   71.35%   +0.41%     
===============================================
  Files               35       35              
  Lines             3221     3278      +57     
===============================================
+ Hits              2285     2339      +54     
- Misses             936      939       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[StokesMIDE]

Some comments, some of which I mentioned over Teams a while back:

• The exact string match for finding pressure/temperature columns is a bit brittle; looking for a substring would probably be more reliable.
• It looks like you are adding a new column to the results; since this is a 'conversion' function (and doesn't modify the input in-place), it might make more sense to replace the pressure column with the new altitude column.
• It might be useful to allow the user to specify a particular column as the pressure column, possibly temperature column as well, in case there is more than one. This isn't a requirement, though.
• The descriptions of the base_* kwargs in the docstring could be more clear.

[pscheidler]

I think this test does the right stuff, but it is going to get a bit harder to maintain as time goes on. What do you think about updating it to use parameterization to take in an input dataframe, parameters, expected result, and maybe an error message, and then essentially looping through the code?
Something like the 2nd example in https://hutchinson-digital.atlassian.net/wiki/spaces/GE/pages/167575568/Best+Practices+for+Python+Testing#Testing-Exceptions we can discuss if it is unclear

[pscheidler]

Some comments, some of which I mentioned over Teams a while back:

• The exact string match for finding pressure/temperature columns is a bit brittle; looking for a substring would probably be more reliable.
• It looks like you are adding a new column to the results; since this is a 'conversion' function (and doesn't modify the input in-place), it might make more sense to replace the pressure column with the new altitude column.
• It might be useful to allow the user to specify a particular column as the pressure column, possibly temperature column as well, in case there is more than one. This isn't a requirement, though.
• The descriptions of the base_* kwargs in the docstring could be more clear.

I agree with this, except number 2, adding a column vs replacing a column. I prefer the way you have it, returning a new dataframe with an added column. We don't expect memory to be an issue in this data, and I think it is easier to drop a column you don't want rather than add in the column you do.

[pscheidler]

This looks a lot better, great work