-
Notifications
You must be signed in to change notification settings - Fork 7
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
Added South Africa and Germany Fieldscapes Converter #43
base: main
Are you sure you want to change the base?
Conversation
aninda-ghosh
commented
Jun 6, 2024
- Added Converter for Fieldscapes subset for Germany. (Currently using local data)
- Added Converter for Fieldscapes subset for South Africa. (Currently using local data)
fiboa_cli/datasets/fs_de_bb.py
Outdated
# Supported protcols: HTTP(S), GCS, S3, or the local file system | ||
|
||
# Local URI added to the repository for initial conversion, Original Source https://beta.source.coop/esa/fusion-competition/ | ||
URI = "/home/byteboogie/fieldscapes/germany/fs_de_bb.gpkg" |
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'm not sure how to proceed with this. It's not really useful for the general public if there's no way to get the source data. Any thoughts from the fieldscapes members?
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.
Chrish asked to keep the local folder for now later we can change this to maybe Source URI? Not sure need extensive discussion on this.
fiboa_cli/datasets/fs_de_bb.py
Outdated
|
||
# License of the data, either | ||
# 1. a SPDX license identifier (including "dl-de/by-2-0" / "dl-de/zero-2-0"), or | ||
LICENSE = "DL-DE->BY-2.0" |
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.
LICENSE = "DL-DE->BY-2.0" | |
LICENSE = "dl-de/by-2-0" |
fiboa_cli/datasets/fs_za_ct.py
Outdated
|
||
# License of the data, either | ||
# 1. a SPDX license identifier (including "dl-de/by-2-0" / "dl-de/zero-2-0"), or | ||
LICENSE = "CC BY-NC-SA 4.0" |
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.
According to https://spdx.github.io/spdx-spec/v2.3/SPDX-license-list/ this should be:
LICENSE = "CC BY-NC-SA 4.0" | |
LICENSE = "CC-BY-NC-SA-4.0" |
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.
Yes I forgot to follow the SPDX format
fiboa_cli/datasets/fs_za_ct.py
Outdated
# Function signature: | ||
# func(column: pd.Series) -> pd.Series | ||
COLUMN_MIGRATIONS = { | ||
"area_m": lambda column: column * 0.0001 |
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.
Is this a left-over from the template?
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.
Yes it was a left over, fixed for now using the latest push
fiboa_cli/datasets/fs_za_ct.py
Outdated
|
||
# Schemas for the fields that are not defined in fiboa | ||
# Keys must be the values from the COLUMNS dict, not the keys | ||
MISSING_SCHEMAS = { |
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.
crop_id and crop_name seem to be missing?!
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.
Fixed the issue with the latest push
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.
A couple of questions and suggested changes.
I'm wondering, is crop_id, crop_name and grid_id a common thing in FieldScapes? If yes, an extension would be good to have.
I guess yes but after last meeting we have removed the grid id on the merged dataset. but crop id and crop_name are not consistent in the subset (So Need Discussion on this further ) |
Happy to help with the extension. |
Please rebase the PR to the latest CLI version, including renaming the URI constant to SOURCES and renaming the parameter cache_file to cache. I tried to push it upstream, but you didn't permit changes to this PR so you need to do these changes yourself. Thanks. |