-
Notifications
You must be signed in to change notification settings - Fork 24
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
Bugfix 2370 aeronetv3 #2372
Bugfix 2370 aeronetv3 #2372
Conversation
@hsoh-u I downloaded a daily averages data file from the website that is similar to what Partha is using: I also downloaded a monthly averages file from the website (it actually had the same name as above, but I renamed it):
@JohnHalleyGotway and @hsoh-u |
Thank you for finding an error with yyyy-mmm. "Month" column exists there instead of "Date(dd:mm:yyyy)" & "Time(hh:mm:ss) columns. Now "Month" column is supported. |
Hi @hsoh-u. Thank you for making these changes. I see that you committed them, but I am still having the same problem and I'm not sure what's going on. Do you see anything obvious?
|
@hsoh-u I think I know what happened. Please don't spend any time on this. I'm pretty sure, but can't test it out now. I'll follow up as soon as I can. |
@hsoh-u Ok. I did get past that initial error, but now I have a new one about the version of MET:
|
I did not apply the bugfix to 10.1 branch yet. It's the next task after approval for merging into develop branch. Would you clone from "bugfix_2370_aeronetv3"?
|
Hi @hsoh-u. Thank you for pointing out my error there! I cleaned everything up and am now compiling and running the proper codebase:
However, I do receive a warning (but not an error) from the monthly averages file:
Do I need to be concerned about that or is that expected? |
The warning (carried over v1 & v2) can be ignored and implies the site_name comes from header (line 2) for version 3. A potential issue will be missing site_name from both header (line 2) and data column. |
@hsoh-u Thank you for letting me know that warning is ok. |
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.
All tests passed, except for the expected change in files. I approve this request. Thank you for all of your work on this @hsoh-u!
Co-authored-by: Howard Soh <hsoh@seneca.rap.ucar.edu> Co-authored-by: Dave Albo <dave@seneca.rap.ucar.edu> Co-authored-by: John Halley Gotway <johnhg@ucar.edu> Co-authored-by: Seth Linden <linden@seneca.rap.ucar.edu> Co-authored-by: johnhg <johnhg@ucar.edu> Co-authored-by: Lisa Goodrich <lisag@seneca.rap.ucar.edu> Co-authored-by: jprestop <jpresto@ucar.edu> Co-authored-by: MET Tools Test Account <met_test@seneca.rap.ucar.edu> Co-authored-by: j-opatz <59586397+j-opatz@users.noreply.github.com> Co-authored-by: George McCabe <23407799+georgemccabe@users.noreply.github.com> Co-authored-by: Julie Prestopnik <jpresto@seneca.rap.ucar.edu> Co-authored-by: Jonathan Vigh <jvigh@ucar.edu> Co-authored-by: hsoh-u <hsoh@ucar.edu> Co-authored-by: bikegeek <3753118+bikegeek@users.noreply.github.com> Co-authored-by: davidalbo <dave@ucar.edu> Co-authored-by: Seth Linden <linden@ucar.edu> Co-authored-by: lisagoodrich <33230218+lisagoodrich@users.noreply.github.com> Co-authored-by: Daniel Adriaansen <dadriaan@ucar.edu>
AERONET V3 input files with the 7 lines header were supported before. The fix is to support 6 lines header (without site_name at line 2). The concatenation of multiple AERONET V3 files should be 6 lines header (can be done by deleting line 2 if 7 lines header).
Note: removed the trailing spaces at aeronet_handler.cc
sanity checking: the site_name at the line 2 and the site_name at the first data column should match.
Expected Differences
Do these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:
Pull Request Testing
existing unittest: no changes with existing AERONET V3 and old formats.
Tested with the sample files at /d1/personal/hsoh/data/discussions/discussions_1888
/d1/personal/hsoh/data/discussions/discussions_1888/GSFC_20221202.lev15.txt: contains just one site with 6 lines header
/d1/personal/hsoh/data/discussions/discussions_1888/20221202.lev15.org.txt: contains multiple sites with 6 lines header
/d1/personal/hsoh/data/discussions/discussions_1888/20221202.lev15.txt: reduced from 20221202.lev15.org.txt for unittest
test with other 6 line herder inputs
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [No]
Do these changes include sufficient testing updates? [Yes]
An unit test was added
If yes, describe the new output and/or changes to the existing output:
One more output: aeronet/20221202.v3.lev15.nc
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s)
Select: Organization level software support Project or Repository level development cycle Project
Select: Milestone as the version that will include these changes