-
Notifications
You must be signed in to change notification settings - Fork 388
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
LandCover.ai: add GeoDataset version #1126
LandCover.ai: add GeoDataset version #1126
Conversation
@microsoft-github-policy-service agree |
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.
This looks correct modulo some documentation hiccups. Let me mull over the class names. Some options:
- LandCoverAINonGeo and LandCoverAIGeo
- LandCoverAI and LandCoverAIGeo
- LandCoverAIBenchmark and LandCoverAIUncurated
I think this will be the first dataset where we have both a geo and non-geo version, so we have an opportunity to set a new standard naming scheme.
For naming, I prefer something like |
I think @isaaccorley said he prefers LandCoverAI for non-geo and LandCoverAIGeo for geo. Not sure how to come to agreement on the names. Would be great if we can decide on an approach that is consistent for all datasets, there are many others that could be both geo and non-geo. |
Yeah on second thought I like that (LandCoverAI for non-geo and LandCoverAIGeo for geo) a bit better:
|
The base class can be called |
Thank you guys for the review and a valuable name discussion. |
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
* Added LandCover.ai geo and non-geo datasets * Updated documentation * Using the new classes in tests * Fixes for documentation * Added tests for geo dataset * Improvements according to the review * Small fixes * Documentation fixes * Small documentation fixes * Update docs/api/geo_datasets.csv Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com> * Fix missing line of coverage --------- Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
This PR renames LandCoverAI class to LandCoverAINonGeo (benchmark dataset, created with
split.py
script) and introduces the abstract base LandCoverAI class and LandCoverAIGeo (raster dataset).This is my proposition, but I am open to the discussion of how the structure, inheritance, and naming should look.