Skip to content
This repository was archived by the owner on Feb 7, 2020. It is now read-only.

Water meters - #1

Open
anton-br wants to merge 61 commits into
masterfrom
water_meters
Open

Water meters#1
anton-br wants to merge 61 commits into
masterfrom
water_meters

Conversation

@anton-br

@anton-br anton-br commented Jan 9, 2018

Copy link
Copy Markdown

No description provided.

Comment thread meters/batch/meter_batch.py Outdated
@action
@inbatch_parallel(init='indices', src='images', post='assemble')
def normalize_images(self, ind, src='images'):
""" Normalize pixel values from (0, 255) to (0, 1).

@GregoryIvanov GregoryIvanov Jan 17, 2018

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

pixels' values

Comment thread meters/batch/meter_batch.py Outdated
from ..dataset.dataset import ImagesBatch, action, inbatch_parallel, any_action_failed

class MeterBatch(ImagesBatch):
"""Class to create batch with water meter"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Кажется, лучше написать Water meters' batch class

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Причем в ридми указывалось, что класс подойдет для любых счетчиков

Comment thread meters/batch/meter_batch.py Outdated
dataset index

src : str
the name of the placeholder with data

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

data placeholder's name

Comment thread meters/batch/meter_batch.py Outdated
Parameters
----------
ind : str or int
dataset index

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dataset's index

Comment thread meters/batch/meter_batch.py Outdated
Returns
-------
array with indices from batch"""
_ = args, kwargs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kwargs тут лишние, так как используются дальше. args нигде не использутся (и от класса никто не наследуется), нужны ли они вообще?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

лишние kwargs и args удалил везде

Comment thread meters/batch/meter_batch.py Outdated
dataset index

src : str
the name of the placeholder with data

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

data placeholder's name

Comment thread meters/batch/meter_batch.py Outdated

@action
@inbatch_parallel(init='_init_component', src='images', dst='bbox', target='threads')
def crop_to_bbox(self, ind, *args, src='images', dst='bbox', **kwargs):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

*args лучше поставить непосредственно перед kwargs

Comment thread meters/batch/meter_batch.py Outdated

dst : str
the name of the placeholder in witch the result will be recorded"""
_ = args, kwargs

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А нужны ли в этой функции args и kwargs?

Comment thread meters/batch/meter_batch.py Outdated
_ = args, kwargs
image = self.get(ind, src)
coord_str = self.get(ind, 'coordinates')
x, y, width, height = [int(val) for val in coord_str.split()]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

x, y, width, height = map(int, coord_str.split())

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

исправил

Comment thread meters/batch/meter_batch.py Outdated
return normalize_image

def _init_component(self, *args, **kwargs):
"""Create and preallocate a new attribute with the name ``dst`` if it

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Create a new attribute with the name specified by kwargs['dst'] and preallocate memory for it

Comment thread meters/batch/meter_batch.py Outdated

def _init_component(self, *args, **kwargs):
"""Create and preallocate a new attribute with the name ``dst`` if it
does not exist and return batch indices

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

batch's indices

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

исправил ошибки в английском везде

Comment thread meters/batch/meter_batch.py Outdated
@action
@inbatch_parallel(init='_init_component', src='images', dst='bbox', target='threads')
def crop_to_bbox(self, ind, *args, src='images', dst='bbox', **kwargs):
"""Create cropped attr with crop image use ``coordinates``

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Crop area from image using coordinates attribute

Comment thread meters/batch/meter_batch.py Outdated
Parameters
----------
ind : str or int
dataset index

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

dataset's index

Comment thread meters/batch/meter_batch.py Outdated
_ = args, kwargs
i = self.get_pos(None, src, ind)
label = getattr(self, src)[i]
more_label = np.array([int(i) for i in label.replace(',', '')] + [None])[:-1]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

я бы пояснил, зачем нужен None и [:-1]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

добавил коммент

Comment thread meters/batch/meter_batch.py Outdated
src : str
data placeholder's name
dst : str
the name of the placeholder's in witch the result will be recorded

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

which, а вообще, по-моему, in which можно заменить на where
of the placeholder's - притяжательный падеж ни к чему, если перед этим использован предлог of
а вообще, не очень понятно, что за placeholder?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

заменил на components, так правильнее

@GregoryIvanov GregoryIvanov left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Check your English.
Check docstrings.
Don't use 'we' in instructions

Comment thread README.md Outdated

# About Meters

Meters has two module: [``batch``](https://github.com/analysiscenter/meters/tree/master/meters/batch) and [``pipelines``](https://github.com/analysiscenter/meters/tree/master/meters/pipelines)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

two modules

Comment thread docs/intro/batch.rst Outdated

* ``images``, input images
* ``labels``, targets for images - array of strings with numbers
* ``coordinates``, array with four numbers - coordinates of one of the top left corner of the bbox, height and width

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

-- coordinates of the top-left corner, height and width of the bounding box

Comment thread docs/intro/batch.rst Outdated
* ``images``, input images
* ``labels``, targets for images - array of strings with numbers
* ``coordinates``, array with four numbers - coordinates of one of the top left corner of the bbox, height and width
* ``display``, array with images cropped by ``coordinates``

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

coordinates don't crop anything.

Array with cropped meters' values or smth like that

Comment thread docs/intro/batch.rst Outdated
* ``display``, array with images cropped by ``coordinates``
* ``digits``, array with ``num_split`` numbers from the meter.

Actions of MeterBatch allows to e.g.:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

allows to:

Comment thread docs/intro/batch.rst Outdated
Actions of MeterBatch allows to e.g.:

* load images from blosc formats and labels from csv format
* crop images by coordinates

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would write like this : "crop bounding box's area from an image"

* save prediction to variable named ``prediction``.

Parameters
----------

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same story as for simple_train

Comment thread meters/utils/format_data.py Outdated
import numpy as np

def main():
"""Convert labels and coordinates from csv to normal format. Normal is ambiguous format"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Very confusing comment

Comment thread meters/utils/format_data.py Outdated
raise ValueError("Missing required argument '-s'")

def format_labels(src):
"""Convert labels from csv to normat format"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the same story

Comment thread meters/utils/format_data.py Outdated
return labels

def format_coordinates(src_coord, labels):
"""Convert coordinates from csv to normat format"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

confusing

Comment thread meters/utils/format_data.py Outdated
return data

def format_data(src_data):
"""Convert data from csv to normat format"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

confusing

Comment thread meters/__init__.py Outdated
__version__ = '0.1.0'


if sys.version_info < (3, 5):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not needed, as dataset will raise an exception even earlier

"""Batch class for water meter task"""
import numpy as np

from ..dataset.dataset import ImagesBatch, action, inbatch_parallel, any_action_failed, DatasetIndex

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

one dataset is not enough?

@anton-br anton-br Jan 31, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, pylint can not find imported modules, like ImagesBatch and etc.

----------
results : array
loaded data
Returns

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add blank line before

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done, i corrected all comments.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants