Refactor: Carbon Pools Package Imports #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* docker-compose works fine and can enter docker container locally. Haven't tried running the model with updated GDAL and Python yet. * Successfully installs the required Python packages. Haven't tried running test tile yet. * Runs test tile 00N_000E. Didn't check that outputs were correct but did verify that the output rasters load and have values in ArcMap.
* Froze all dependencies in `requirements.txt` to their current version. Also, added testing folder and file but haven't tried using them yet. * Used pylint on mp_create_carbon_pools.py. Addressed pretty much all the messages I wanted to. * Continued delinting in create_carbon_pools. Changed all obvious print statements in mp_create_carbon_pools.py and create_carbon_pools.py to fprint. Added docstrings to each function. Testing carbon pool creation for 00N_000E seems to work fine. * Experimenting with setting some variables as global so that I don't have to pass them as arguments: sensit_type, no_upload, save_intermediates, etc. For saving and modifying variables between files, this page seems to be helpful: https://thewebdev.info/2021/10/19/how-to-use-global-variables-between-files-in-python/#:~:text=To%20use%20global%20variables%20between%20files%20in%20Python%2C%20we%20can,reference%20the%20global%20variable%20directly.&text=We%20import%20the%20settings%20and,Then%20we%20call%20settings. * Testing global variables with no_upload. I seem to be able to reset the global variable from run_full_model.py, including in the log. Need to make sure this is actually carrying through to uploading. * Added global variables to constants_and_names.py and top of run_full_model.py. * Changed run_full_model.py through carbon pool step. * Changed carbon pool creation to use global variables. Decided to have functions pass carbon_pool_extent because it's a key parameter of carbon pool creation. * Changed all model stages to use global variables from the command line. Still testing that I didn't break anything in local runs. * Changed some universal_util.py functions to use the global variables instead of passing arguments to them. * Starting to change print statements to f'' print statements throughout the model. * Changed to f print statements for model extent and forest age category steps. * Changed to f print statements for entire removals model. * Changed to f print statements for carbon, emissions, and analyses. Haven't changed in universal_util or constants_and_names. Haven't checked if everything is working alright. * Changed to f print statements for universal_util.py. Didn't change arguments to gdal commands for the most part, though. * Used pylint on all regular model steps and run_full_model.py. Fixed most message that weren't about importing, too many variables, too many statements, or too many branches. I'll work on those structural issues later. * Testing 00N_000E locally with linting of run_full_model.py and all model stages through net flux. Going to try running it on an ec2 instance now. * 00N_000 works in a full local run and 00N_020E works in a full ec2 run. I've linted enough for now.
* Added a command line argument `--single-processor` or `-sp` to run_full_model.py and each model step through net flux that sets whether the tile processing is done with the multiprocessing module or not. This involved adding another if...else statement (or sometimes statements) to each step to have it use the correct processing route. Also changed readme.md to add the new argument. * Ran 00N_000E locally for all model steps with single and multiprocessing options to make sure both still worked after this reconfiguration. Both worked. Single processing took (no uploading of outputs): 1 hour 23 minutes Multi-processing took (no uploading of outputs): 1 hour 11 minutes
PEP 8: https://peps.python.org/pep-0008/#imports - Absolute imports are recommended
running: $> `python -m carbon_pools.create_carbon_pools` succeeds in importing the modules. So, I don't think this is necessary any longer.
PEP-8: https://peps.python.org/pep-0008/#imports Imports should be grouped in the following order: 1. Standard library imports. 2. Related third party imports. 3. Local application/library specific imports. You should put a blank line between each group of imports.
running: $> `python -m carbon_pools.create_soil_C` succeeds in importing the modules. So, I don't think this is necessary any longer.
PEP-8: https://peps.python.org/pep-0008/#imports Imports should be grouped in the following order: 1. Standard library imports. 2. Related third party imports. 3. Local application/library specific imports. You should put a blank line between each group of imports.
running: $> `python -m carbon_pools.mp_create_carbon_pools` succeeds in importing the modules. So, I don't think this is necessary any longer.
PEP-8: https://peps.python.org/pep-0008/#imports Explicit relative imports are an acceptable alternative to absolute imports, especially when dealing with complex package layouts where using absolute imports would be unnecessarily verbose.
PEP-8: https://peps.python.org/pep-0008/#imports Imports should be grouped in the following order: 1. Standard library imports. 2. Related third party imports. 3. Local application/library specific imports. You should put a blank line between each group of imports.
PEP-8: https://peps.python.org/pep-0008/#imports Explicit relative imports are an acceptable alternative to absolute imports, especially when dealing with complex package layouts where using absolute imports would be unnecessarily verbose.
running: $> `python -m carbon_pools.mp_create_soil_C` succeeds in importing the modules. So, I don't think this is necessary any longer.
PEP-8: https://peps.python.org/pep-0008/#imports Imports should be grouped in the following order: 1. Standard library imports. 2. Related third party imports. 3. Local application/library specific imports. You should put a blank line between each group of imports.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As promised, I did some
importinvestigation and cleanup.Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Import statements are messy and append to
sys.pathduring loading.Issue Number: N/A
What is the new behavior?
Imports follow PEP-8 and no longer require
sys.pathmodificationDoes this introduce a breaking change?
Other information