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

Remove deprecated properties from dataset class #1025

Merged

Conversation

yd8534976
Copy link
Contributor

@yd8534976 yd8534976 commented May 7, 2020

What this patch does to fix the issue.

Remove some deprecated properties from dataset class. These feature will be done in

class DatasetIterator:
available_subsets = ["train", "validation"]
"""docstring for DatasetIterator."""
def __init__(self, dataset, enable_prefetch=False, seed=0, local_rank=-1):
self.dataset = dataset
self.enable_prefetch = enable_prefetch
self.seed = seed
if issubclass(dataset.__class__, TFDSMixin):
self.enable_prefetch = False
self.reader = _TFDSReader(self.dataset, local_rank)
else:
if self.enable_prefetch:
self.prefetch_result_queue = queue.Queue(maxsize=200)
self.prefetcher = _MultiProcessDatasetPrefetchThread(self.dataset, self.prefetch_result_queue, seed)
self.prefetcher.start()
print("ENABLE prefetch")
else:
self.reader = _SimpleDatasetReader(self.dataset, seed)
print("DISABLE prefetch")
@property
def num_per_epoch(self):
return self.dataset.num_per_epoch
@property
def classes(self):
return self.dataset.classes
@property
def num_classes(self):
return self.dataset.num_classes
@property
def num_max_boxes(self):
return self.dataset.num_max_boxes
@property
def label_colors(self):
return self.dataset.label_colors
def extend_dir(self):
return self.dataset.extend_dir
def __iter__(self):
return self
def __next__(self):
if self.enable_prefetch:
(images, labels) = self.prefetch_result_queue.get()
else:
images, labels = self.reader.read()
return images, labels
def feed(self):
return self.__next__()
def __len__(self):
return len(self.dataset)
def update_dataset(self, indices):
"""Update own dataset by indices."""
# do nothing so far
return
def get_shuffle_index(self):
"""Return list of shuffled index."""
random_state = np.random.RandomState(self.seed)
random_indices = list(range(self.num_per_epoch))
random_state.shuffle(random_indices)
print("Shuffle {} train dataset with random state {}.".format(self.__class__.__name__, self.seed))
print(random_indices[0:10])
self.seed += 1
return random_indices
.

Link to any relevant issues or pull requests.

@blueoil-butler blueoil-butler bot added the CI: auto-run Run CI automatically label May 7, 2020
@yd8534976 yd8534976 changed the title [WIP] Remove deprecated properties from dataset class Remove deprecated properties from dataset class May 7, 2020
@bo-code-review-bot
Copy link

This PR needs Approvals as follows.

  • Ownership Approval for / from iizukak, tkng, ruimashita
  • Readability Approval for Python from tkng, tsawada, tfujiwar

Please choose reviewers and requet reviews!

Click to see how to approve each reviews

You can approve this PR by triggered comments as follows.

  • Approve all reviews requested to you (readability and ownership) and LGTM review
    Approval, LGTM

  • Approve all ownership reviews
    Ownership Approval or OA

  • Approve all readability reviews
    Readability Approval or RA

  • Approve specified review targets

    • Example of Ownership Reviewer of /: Ownership Approval for / or OA for /
    • Example of Readability Reviewer of Python: Readability Approval for Python or RA for Python
  • Approve LGTM review
    LGTM

See all trigger comments

Please replace [Target] to review target

  • Ownership Approval
    • Ownership Approval for [Target]
    • OA for [Target]
    • Ownership Approval
    • OA
    • Approval
  • Readability Approval
    • Readability Approval for [Target]
    • RA for [Target]
    • [Target] Readability Approval
    • [Target] RA
    • Readability Approval
    • RA
    • Approval
  • LGTM
    • LGTM
    • lgtm

@yd8534976 yd8534976 requested a review from tfujiwar May 7, 2020 06:49
Copy link
Contributor

@tfujiwar tfujiwar left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up.

def _get_index(self, counter):
return self.indices[counter]

def _shuffle(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is called in dataset_iterator.py but I think it's not needed. Could you refactor it also?

@yd8534976 yd8534976 added the CI: test-all Run all tests once label May 7, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 7, 2020
@yd8534976 yd8534976 added the CI: test-all Run all tests once label May 7, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 7, 2020
@yd8534976 yd8534976 added the CI: test-blueoil Run blueoil test once label May 7, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-blueoil Run blueoil test once label May 7, 2020
@yd8534976 yd8534976 added the CI: test-all Run all tests once label May 7, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 7, 2020
@iizukak iizukak added the CI: test-all Run all tests once label May 8, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 8, 2020
Copy link
Member

@iizukak iizukak left a comment

Choose a reason for hiding this comment

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

Thanks.
Good refactoring.
OA

@yd8534976 yd8534976 added the CI: test-all Run all tests once label May 8, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 8, 2020
@yd8534976 yd8534976 force-pushed the remove_dataset_deprecated_properties branch from fefd97d to 0fa476a Compare May 8, 2020 09:01
@yd8534976 yd8534976 added the CI: test-all Run all tests once label May 8, 2020
@blueoil-butler blueoil-butler bot removed the CI: test-all Run all tests once label May 8, 2020
@yd8534976 yd8534976 requested a review from tfujiwar May 10, 2020 20:22
Copy link
Contributor

@tfujiwar tfujiwar left a comment

Choose a reason for hiding this comment

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

Readability Approval
Thank you!

@yd8534976 yd8534976 merged commit 5fa1edf into blue-oil:master May 11, 2020
@yd8534976 yd8534976 deleted the remove_dataset_deprecated_properties branch May 11, 2020 04:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI: auto-run Run CI automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants