Skip to content
This repository has been archived by the owner on Jul 2, 2021. It is now read-only.

Add CityscapesTestImageDataset #440

Merged
merged 10 commits into from
Oct 7, 2017
Merged

Conversation

mitmul
Copy link
Member

@mitmul mitmul commented Oct 4, 2017

Please merge #437 first.
This PR adds CityscapesTestImageDataset class.

@yuyu2172 yuyu2172 self-assigned this Oct 5, 2017
@yuyu2172
Copy link
Member

yuyu2172 commented Oct 5, 2017

Can you resolve conflicts?

@mitmul
Copy link
Member Author

mitmul commented Oct 5, 2017

Rebased to master

Copy link
Member

@yuyu2172 yuyu2172 left a comment

Choose a reason for hiding this comment

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

Can you add doc link to docs/source/dataset.rst?

@@ -3,6 +3,8 @@
from chainercv.datasets.camvid.camvid_dataset import camvid_label_names # NOQA
from chainercv.datasets.camvid.camvid_dataset import CamVidDataset # NOQA
from chainercv.datasets.cityscapes.cityscapes_semantic_segmentation_dataset import CityscapesSemanticSegmentationDataset # NOQA
from chainercv.datasets.cityscapes.cityscapes_test_image_dataset import CityscapesTestImageDataset # NOQA
from chainercv.datasets.cityscapes.cityscapes_utils import cityscapes_labels # NOQA
Copy link
Member

Choose a reason for hiding this comment

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

Can you delete this line? This may confuse users because there is not sufficient description about this object.
We are only exposing label_names and label_colors directly below chainercv.datasets.


class CityscapesTestImageDataset(dataset.DatasetMixin):

"""Dataset class for test images of `Cityscapes dataset`_.
Copy link
Member

Choose a reason for hiding this comment

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

Image dataset for test split of `Cityscapes dataset`_.

This is more consistent with other Image dataset (e.g. ADE20KTestImageDataset).

@mitmul
Copy link
Member Author

mitmul commented Oct 6, 2017

@yuyu2172 Fixed.

@yuyu2172
Copy link
Member

yuyu2172 commented Oct 6, 2017

Please add the dataset to dataset.rst.

@mitmul
Copy link
Member Author

mitmul commented Oct 6, 2017

oops, missed it. added.

@yuyu2172
Copy link
Member

yuyu2172 commented Oct 6, 2017

LGTM

yuyu2172
yuyu2172 previously approved these changes Oct 6, 2017
@yuyu2172
Copy link
Member

yuyu2172 commented Oct 6, 2017

Sorry. I found that the test fails.
Please fix that.

@yuyu2172 yuyu2172 dismissed their stale review October 6, 2017 11:41

failing test

@yuyu2172
Copy link
Member

yuyu2172 commented Oct 6, 2017

Also, I think we should add tearDown method.
I totally missed them. Sorry!

@mitmul
Copy link
Member Author

mitmul commented Oct 6, 2017

@yuyu2172 Added teardown method.

def setUp(self):
self.temp_dir = tempfile.mkdtemp()
img_dir = os.path.join(
self.temp_dir, 'leftImg8bit/{}/aachen'.format(self.split))
Copy link
Member

Choose a reason for hiding this comment

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

self.split raises an error

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, fixed.

@yuyu2172
Copy link
Member

yuyu2172 commented Oct 7, 2017

LGTM

@yuyu2172 yuyu2172 merged commit c759bc1 into chainer:master Oct 7, 2017
@yuyu2172 yuyu2172 added this to the v0.8 milestone Dec 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants