Skip to content
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

fix: Type error in GCBM API causes (fatal) - main(306) - Value too large. error. #197

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions local/rest_api_gcbm/preprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ def get_config_templates(input_dir):
# current hack is to drop the last five characters, but thats very fragile
def get_modules_cbm_config(input_dir):
with open(f"{input_dir}/templates/modules_cbm.json", "r+") as modules_cbm_config:
disturbances = []
data = json.load(modules_cbm_config)
for file in os.listdir(f"{input_dir}/disturbances/"):
disturbances.append(
file.split(".")[0][:-5]
) # drop `_moja` to match modules_cbm.json template
disturbances = [file.split(".")[0][:-5] for file in os.listdir(f"{input_dir}/disturbances/")] # drop `_moja` to match modules_cbm.json template
modules_cbm_config.seek(0)
data["Modules"]["CBMDisturbanceListener"]["settings"]["vars"] = disturbances
json.dump(data, modules_cbm_config, indent=4)
Expand Down Expand Up @@ -75,13 +71,13 @@ def get_provider_config(input_dir):
cellLonSize = []
paths = []

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/disturbances/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/disturbances/")):
for file in files:
fp = os.path.join(root, file)
Rasters.append(fp)
paths.append(fp)

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/classifiers/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/classifiers/")):
for file in files:
fp1 = os.path.join(root, file)
Rasters.append(fp1)
Expand Down Expand Up @@ -136,7 +132,7 @@ def get_provider_config(input_dir):
# should be able to accept variable number of inputs, but requires
# means for user to specify/verify correct ["attributes"]
def get_input_layers():
for root, dirs, files in os.walk(
for root, _, files in os.walk(
os.path.abspath(f"{input_dir}/miscellaneous/")
):
for file in files:
Expand All @@ -153,15 +149,15 @@ def get_input_layers():
) as json_file:
dictionary["layer_type"] = "GridLayer"
dictionary["layer_data"] = "Int16"
dictionary["nodata"] = nodatam[1]
dictionary["nodata"] = 32767
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Namyalg @Janvi-Thakkar @SanjaySinghRajpoot @YashKandalkar - what do you think of this solution?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, as a temporary fix, we can probably go ahead with this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might also need a comment explaining this temporary solution, so that other contributors know why the hard coded value is there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Freeman-kuch - can you please add documentation to this patch? Using a hard coded number is not a good long term solution, but it will do for now.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aornugent I'm not so sure i understand what you mean about adding documentation, can you help out ?

Copy link
Contributor

@aornugent aornugent Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean leaving a comment in the code to explain your change. Without documentation it's hard for others to understand why this value is set.

Maybe something like:

# FIXME: hard coded nodata value is a temporary solution, 
but should instead come from the input data source. 

json.dump(dictionary, json_file, indent=4)

with open(
f"{input_dir}/mean_annual_temperature_moja.json", "w", encoding="utf8"
) as json_file:
dictionary["layer_type"] = "GridLayer"
dictionary["layer_data"] = "Float32"
dictionary["nodata"] = nodatam[0]
dictionary["nodata"] = 32767
json.dump(dictionary, json_file, indent=4)

with open(
Expand Down Expand Up @@ -313,31 +309,31 @@ def get_study_area():

get_study_area()

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/disturbances/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/disturbances/")):
for file in files:
fp = os.path.join(root, file)
Rasters.append(fp)
paths.append(fp)

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/classifiers/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/classifiers/")):
for file in files:
fp1 = os.path.join(root, file)
Rasters.append(fp1)
paths.append(fp1)

for root, dirs, files in os.walk(
for root, _, files in os.walk(
os.path.abspath(f"{input_dir}/miscellaneous/")
):
for file in files:
fp2 = os.path.join(root, file)
paths.append(fp2)

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/templates/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/templates/")):
for file in files:
fp3 = os.path.join(root, file)
paths.append(fp3)

for root, dirs, files in os.walk(os.path.abspath(f"{input_dir}/db/")):
for root, _, files in os.walk(os.path.abspath(f"{input_dir}/db/")):
for file in files:
fp4 = os.path.join(root, file)
paths.append(fp4)
Expand Down