-
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 2428 python from env main v11.0 #2443
Bugfix 2428 python from env main v11.0 #2443
Conversation
…_util.cc & python3_util.h
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.
The changes to the logs that list the version of Python used as described in this comment weren't implemented here.
I think it would be best for @DanielAdriaansen to test running with these changes to ensure the output matches what he expects.
How to get the compile time python exe? |
I'm sorry, I didn't realize this was for main_v11.0. I don't think any log changes are needed.
Modified read_tmp_point_nc.py & write_tmp_point_nc.py (github and /d1/personal/hsoh/git/pull_request/MET_bugfix_2428_python_from_env_main_v11.0).
|
@hsoh-u should I am running this command on seneca:
But get this error:
|
It depends on MET_BASE setting. MET adds MET_BASE/python and MET_BASE/wrappers into the system path for python. If MET_BASE is not set, it fails to find "met_point_obs.py" which is in MET_BASE/python directory ("/python" and "/wrappers" are added to the system path). Please check if this helps:
|
OK, that helps thank you Howard. Now I am getting this error:
I think this is related to #2386 is that right? It looks like NumPy is listed already as a dependency for MET Python Embedding, so I think this is OK, and I will change to a Python environment that has NumPy because apparently I am adding a note on #2414 to suggest unifying the Python requirements for METplus wrappers and the requirements for a Python to use when building MET for Python Embedding support. |
@georgemccabe regarding Howard's comment:
Does METplus wrappers set |
No, it's not.
|
Is there an open issue to log |
Not sure without METplus configuration. I set MET_BASE manually as needed at seneca. |
I believe MET_BASE is set to a default value of where MET is installed. The wrappers do not override this value. You shouldn't have to set MET_BASE to include the python directory in the path. It is added in other files by using the relative path from the script. See: MET/data/wrappers/write_tmp_dataplane.py Lines 25 to 26 in eb987a4
|
@hsoh-u can you please add the code @georgemccabe sent from |
I don't think this is an issues. The log already exists for python embedding of gridded data (MET v11.1 development version) or the existing log message should be changed as @georgemccabe mentioned this comment. The same log should be added for the python embedding of the point observation data. But it can not be added to the 11.0 branch for both (python embedding of gridded and point data). |
OK, that makes sense. Then will this PR also be merged into MET v11.1 development version? |
Updated at github repository Sorry my script does not allow new build for Pull Request to the different location.
|
I performed the following testing on seneca with the new changes. TEST 1
This worked as expected, and worked previously. TEST 2
This would also have worked previously, but would have silently used the same Python as TEST 1, instead of The output from TEST 2 now correctly informs me MET is using
Howard also added additional changes so that a user does not need to set In future versions, the addition of In addition, @georgemccabe shared with me this change: |
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 approve.
Merging this PR into main_v11.0 based on @DanielAdriaansen's approving review. |
Co-authored-by: MET Tools Test Account <[email protected]> Co-authored-by: Howard Soh <[email protected]> Co-authored-by: jprestop <[email protected]> Co-authored-by: hsoh-u <[email protected]> Co-authored-by: George McCabe <[email protected]> fix #2389 main_v11.0 flowchart (#2391) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405) fix #2380 main_v11.0 override (#2381) fix #2408 main_v11.0 empty config (#2409) fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403) fix #2415 main_v11.0 modulefiles (#2416) fix #2412 main_v11.0 climo (#2420) fix #2426 main_v11.0 buoy (#2432) fix #2437 main_v11.0 convert (#2438) fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix 2428 python from env main v11.0 (#2443) fix 2428 python csv input (#2450) fix #2452 main_v11.0 airnow (#2453) fix #2402 main_v11.0 sonarqube (First PR) (#2447)
Co-authored-by: MET Tools Test Account <[email protected]> Co-authored-by: Howard Soh <[email protected]> Co-authored-by: John Halley Gotway <[email protected]> Co-authored-by: jprestop <[email protected]> Co-authored-by: hsoh-u <[email protected]> Co-authored-by: George McCabe <[email protected]> fix #2389 main_v11.0 flowchart (#2391) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405) fix #2380 main_v11.0 override (#2381) fix #2408 main_v11.0 empty config (#2409) fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403) fix #2415 main_v11.0 modulefiles (#2416) fix #2412 main_v11.0 climo (#2420) fix #2426 main_v11.0 buoy (#2432) fix #2437 main_v11.0 convert (#2438) fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix 2428 python from env main v11.0 (#2443) fix 2428 python csv input (#2450) fix #2452 main_v11.0 airnow (#2453) fix #2402 main_v11.0 sonarqube (First PR) (#2447) fix #2449 main_v11.0 pdf (#2465) fix 2428 python csv input (#2467)
Co-authored-by: MET Tools Test Account <[email protected]> Co-authored-by: Howard Soh <[email protected]> Co-authored-by: John Halley Gotway <[email protected]> Co-authored-by: jprestop <[email protected]> Co-authored-by: hsoh-u <[email protected]> Co-authored-by: George McCabe <[email protected]> fix #2389 main_v11.0 flowchart (#2391) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405) fix #2380 main_v11.0 override (#2381) fix #2408 main_v11.0 empty config (#2409) fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403) fix #2415 main_v11.0 modulefiles (#2416) fix #2412 main_v11.0 climo (#2420) fix #2426 main_v11.0 buoy (#2432) fix #2437 main_v11.0 convert (#2438) fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix 2428 python from env main v11.0 (#2443) fix 2428 python csv input (#2450) fix #2452 main_v11.0 airnow (#2453) fix #2402 main_v11.0 sonarqube (First PR) (#2447) fix #2449 main_v11.0 pdf (#2465) fix 2428 python csv input (#2467)
Co-authored-by: MET Tools Test Account <[email protected]> Co-authored-by: Howard Soh <[email protected]> Co-authored-by: John Halley Gotway <[email protected]> Co-authored-by: jprestop <[email protected]> Co-authored-by: hsoh-u <[email protected]> Co-authored-by: George McCabe <[email protected]> Co-authored-by: lisagoodrich <[email protected]> Co-authored-by: Stephen Herbener <[email protected]> fix #2389 main_v11.0 flowchart (#2391) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405) fix #2380 main_v11.0 override (#2381) fix #2408 main_v11.0 empty config (#2409) fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403) fix #2415 main_v11.0 modulefiles (#2416) fix #2412 main_v11.0 climo (#2420) fix #2426 main_v11.0 buoy (#2432) fix #2437 main_v11.0 convert (#2438) fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix 2428 python from env main v11.0 (#2443) fix 2428 python csv input (#2450) fix #2452 main_v11.0 airnow (#2453) fix #2402 main_v11.0 sonarqube (First PR) (#2447) fix #2449 main_v11.0 pdf (#2465) fix 2428 python csv input (#2467) fix #2514 main_v11.0 clang (#2556) fix #2575 main_v11.0 python_convert (#2577) fix 2596 main v11.0 rpath compilation (#2600) fix 2596 main v11.0 rpath compilation (#2613) fix #2644 main_v11.0 percentile (#2645)
…2770) Co-authored-by: MET Tools Test Account <[email protected]> Co-authored-by: Howard Soh <[email protected]> Co-authored-by: John Halley Gotway <[email protected]> Co-authored-by: jprestop <[email protected]> Co-authored-by: hsoh-u <[email protected]> Co-authored-by: George McCabe <[email protected]> Co-authored-by: lisagoodrich <[email protected]> Co-authored-by: Stephen Herbener <[email protected]> fix #2389 main_v11.0 flowchart (#2391) fix definitions of G172 and G220 based on comments in NOAA-EMC/NCEPLIBS-w3emc#157. (#2405) fix #2380 main_v11.0 override (#2381) fix #2408 main_v11.0 empty config (#2409) fix #2390 main_v11.0 fix compiling hdf5 with zlib and handle NetCDF-C zip (#2403) fix #2415 main_v11.0 modulefiles (#2416) fix #2412 main_v11.0 climo (#2420) fix #2426 main_v11.0 buoy (#2432) fix #2437 main_v11.0 convert (#2438) fix for main_v11.0, for #2437, forgot one reference to the search_parent for a dictionary lookup. fix 2428 python from env main v11.0 (#2443) fix 2428 python csv input (#2450) fix #2452 main_v11.0 airnow (#2453) fix #2402 main_v11.0 sonarqube (First PR) (#2447) fix #2449 main_v11.0 pdf (#2465) fix 2428 python csv input (#2467) fix #2514 main_v11.0 clang (#2556) fix #2575 main_v11.0 python_convert (#2577) fix 2596 main v11.0 rpath compilation (#2600) fix 2596 main v11.0 rpath compilation (#2613) fix #2644 main_v11.0 percentile (#2645)
Expected Differences
If MET_PYTHON_EXE is defined, the met_point_data should be saved as temporary NetCDF file and MET reads the NetCDF file. The log message shows if the temporary file was created. MET deletes the temporary file.
Without the environment variable MET_PYTHON_EXE
With the environment variable MET_PYTHON_EXE
Note: The following log message was not added for 11.0 branch because MET_PYTHON_BIN_EXE was not introduced at 11.0 branch.
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
at seneca:
Test from #2428
Added unit test
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
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]
Unittest "python_point2grid_pb2nc_TMP_user_python" was added. It should produce the same output with python_point2grid_pb2nc_TMP (pb2nc_TMP.nc and pb2nc_TMP_user_python.nc).
If yes, describe the new output and/or changes to the existing output:
No except a new output python/pb2nc_TMP_user_python.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