Correct error in assert_year #37
Conversation
This comment has been minimized.
This comment has been minimized.
GwenaelleSa
left a comment
GwenaelleSa
left a comment
There was a problem hiding this comment.
It works for me as is.
In the info printed with logger, it seems obvious to you that the year column is "year_tile", but is it indicated somewhere? Because it could be logic for someone else that the year column is just "year".
scripts/generate_tilesets.py
Outdated
| except: | ||
| if img_src=='XYZ' and year=='multi-year': | ||
| if year=='multi-year': | ||
| logger.error("Option 'multi-year' chosen but the tile geodataframe does not contain a year column. " |
There was a problem hiding this comment.
"the tile geodataframe does not contain the "year_tile" column."
There was a problem hiding this comment.
Modified but I kept 'year'
scripts/generate_tilesets.py
Outdated
| sys.exit(1) | ||
|
|
||
| if img_src=='WMS': | ||
| logger.error("Connector=WMS, delete the year column in the tile geodataframe") |
There was a problem hiding this comment.
In this case, I don't know if it is necessary to stop the code. Wouldn't it be enough to just warn that the year will not be considered? It would be the same as for the MIL connector and the FOLDER connector only use it for the id decomposition.
I would change the beginning of the message with something like "The connector WMS does not support a specific year, ...".
There was a problem hiding this comment.
Yeah it was my first intention.
But here if the script is not exited, it leads to an error afterward because the id column contains the year info as well. It is produced if a year is provided when running prepare_data.py. So in this case the tile gdf has to be reprocessed without year.
But we can add what you suggest in case a year argument is set in the config file.
There was a problem hiding this comment.
I have changed the message according to your suggestion.
cleherny
left a comment
cleherny
left a comment
There was a problem hiding this comment.
Thanks for your feedback.
Actually a tile gdf with a column year is provided to the script generate_tilesets.py. It is then renamed 'year_tile' to be differenciate from the 'year_label' in generate_tilesets.py. So for the user it is clearer to keep 'year'.
scripts/generate_tilesets.py
Outdated
| except: | ||
| if img_src=='XYZ' and year=='multi-year': | ||
| if year=='multi-year': | ||
| logger.error("Option 'multi-year' chosen but the tile geodataframe does not contain a year column. " |
There was a problem hiding this comment.
Modified but I kept 'year'
scripts/generate_tilesets.py
Outdated
| sys.exit(1) | ||
|
|
||
| if img_src=='WMS': | ||
| logger.error("Connector=WMS, delete the year column in the tile geodataframe") |
There was a problem hiding this comment.
Yeah it was my first intention.
But here if the script is not exited, it leads to an error afterward because the id column contains the year info as well. It is produced if a year is provided when running prepare_data.py. So in this case the tile gdf has to be reprocessed without year.
But we can add what you suggest in case a year argument is set in the config file.
scripts/generate_tilesets.py
Outdated
| sys.exit(1) | ||
|
|
||
| if img_src=='WMS': | ||
| logger.error("Connector=WMS, delete the year column in the tile geodataframe") |
There was a problem hiding this comment.
I have changed the message according to your suggestion.
Analysis Details1 Issue
Coverage and Duplications
Project ID: swiss-territorial-data-lab_object-detector_AYZ4zWIzr7JdaaSXwexc |
The function
assert_yearin the master branch is not working properly. New propositions.