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

Add plotting method for COWC dataset #300

Merged
merged 8 commits into from
Dec 28, 2021
Merged

Conversation

nilsleh
Copy link
Collaborator

@nilsleh nilsleh commented Dec 17, 2021

Adding plotting method for COWC dataset.

COWCCounting:

cowcCounting2

COWCDetection
cowcDetection2

An Example of image with color:

colorCOWC

@adamjstewart
Copy link
Collaborator

  • For counting, can you give an example image with # cars > 0?
  • For detection, it might make more sense for the labels (0/1) to be displayed as Boolean (True/False).

@adamjstewart adamjstewart added the datasets Geospatial or benchmark datasets label Dec 17, 2021
Copy link
Collaborator

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

You might be able to put most of the plotting code in the COWC base class, then the subclasses only need to decide what the label/prediction is. If it makes it easier, we could even consider going back from True/False to 1/0 and then we don't need separate plotting code.

torchgeo/datasets/cowc.py Outdated Show resolved Hide resolved
@adamjstewart
Copy link
Collaborator

This looks good to me, but I'll let others review. I'm starting to flip flop, and I think it might be simpler (less code to maintain) if we go back to your first commit where there's a single plot function for the COWC dataset. Having detection print 1/0 instead of True/False doesn't seem that important to me when compared to the code duplication.

Copy link
Collaborator

@isaaccorley isaaccorley left a comment

Choose a reason for hiding this comment

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

I think this lgtm, although I'm curious why the example plots look to be grayscale? Are the images not RGB?

torchgeo/datasets/cowc.py Outdated Show resolved Hide resolved
torchgeo/datasets/cowc.py Outdated Show resolved Hide resolved
@adamjstewart adamjstewart changed the title plotting method COWC dataset Add plotting method for COWC dataset Dec 18, 2021
@adamjstewart
Copy link
Collaborator

I'm curious why the example plots look to be grayscale? Are the images not RGB?

The images should be RGB, this seems like a bug in the plotting code in this PR.

@nilsleh
Copy link
Collaborator Author

nilsleh commented Dec 19, 2021

I believe that while all images in the dataset have RGB channels, they are not all colored, see the example on the dataset website.

@adamjstewart
Copy link
Collaborator

Can you show one of the colored images from your plotting method?

@adamjstewart adamjstewart added this to the 0.2.0 milestone Dec 19, 2021
@adamjstewart adamjstewart mentioned this pull request Dec 21, 2021
19 tasks
@github-actions github-actions bot added the testing Continuous integration testing label Dec 24, 2021
@adamjstewart
Copy link
Collaborator

I think we should go back to having a single plot method in the superclass instead of adding a lot of extra code just to convert 0/1 to False/True.

@adamjstewart adamjstewart merged commit 45df0f8 into microsoft:main Dec 28, 2021
@adamjstewart adamjstewart added utilities Utilities for working with geospatial data and removed utilities Utilities for working with geospatial data labels Jan 2, 2022
yichiac pushed a commit to yichiac/torchgeo that referenced this pull request Apr 29, 2023
* plotting method cowc dataset

* boolean label for detection

* restructure plotting method

* typo

* label title not as variable

* single plot method in super class

Co-authored-by: Caleb Robinson <calebrob6@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datasets Geospatial or benchmark datasets testing Continuous integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants