WIP ImageDimensionBear: Add ImageDimensionBear to coala-bears #1288

rhemon · 2017-01-10T17:28:23Z

I have made the bear and written unittest for the bear as well. I am doing this PR to confirm what I have done till now is fine.
Another reason for making this WIP is because I still have to handle files for the bear. This is a GlobalBear and doesnt need any file, however I am unable to run the bear without a file. Presently .coafile has to look something like this to run ImageDimensionBear:

[default]
bears = ImageDimensionBear
files = .coafile
img_config = src/*.png 240 240, img/*.jpg 400 400

now here I think files is unnecessary and want to remove this but not sure how to.

Herer's how the it looks when I run the bear: http://pastebin.com/UebXyuiE
& the Test:

(venv) MBPs-MacBook-Pro:coala-bears rhemon$  /Users/mbp13inch/Documents/Workspace/venv/bin/py.test tests/general/ImageDimensionBearTest.py
================================================== test session starts ===================================================
platform darwin -- Python 3.5.2, pytest-3.0.5, py-1.4.32, pluggy-0.4.0
rootdir: /Users/mbp13inch/Documents/Workspace/coala-bears, inifile: setup.cfg
plugins: cov-2.4.0, env-0.6.0, timeout-1.2.0, xdist-1.15.0
timeout: 35.0s method: signal
collected 4 items 

tests/general/ImageDimensionBearTest.py ....

=============================================== 4 passed in 29.30 seconds ================================================

gitmate-bot · 2017-01-10T17:28:36Z

Thanks for your contribution!

Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.

I have rebased properly. Please see this for a tutorial.
I have gone through the commit guidelines and I've followed them:
I have followed the guidelines for the commit shortlog.
I have followed the guidelines for the commit body.
I have included the issue URL.
I have used the Fixes keyword if the commit fixes a real bug and the Closes keyword if it adds a feature/enhancement. See more here.
I have run all the tests and they all pass. That includes that all CI (below) is green - if GitMate issues appear you will have to amend your commits, pushing new commits on top is not sufficient!

As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well!

gitmate-bot · 2017-01-10T17:29:24Z

Comment on 0498502.

Shortlog of the HEAD commit contains 57 character(s). This is 7 character(s) longer than the limit (57 > 50).

GitCommitBear, severity NORMAL, section commit.

jayvdb · 2017-01-11T01:38:32Z

Regarding your general problem of files, we still do want to use files here.
Currently you have one .coafile setting image_config, which include multiple pieces of information, including filenames.

So the first step is splitting image_config into multiple settings.

-    def run(self, image_config):
+    def run(self, files, image_width, image_height):

The benefit is that the user only needs to set files and two numerical values. nice and simple to configure.
(Of course this means your job is harder, as you need to convert files into the linters yaml, but .. that is the job of a bear -- making linting easy)

This approach has one weakness compared to your image_config strategy, which is that each .coafile section can only have one width & height.

The result is that the user needs to create multiple .coafile sections for different sets of images.
The 'cost' is multiple invocations of the ruby interpreter; one for each section.

jayvdb · 2017-01-11T02:49:13Z

Gemfile

@@ -7,3 +7,5 @@ gem 'scss_lint', require: false# require flag is necessary https://github.com/br
 gem "reek"
 gem "puppet-lint"
 gem "csvlint"
+gem "img_checker"
+gem "fastimage"


why do we need this? doesnt img_checker declare a dependency on fastimage?

not sure, but I was having problem in running it, so I added it, and then it started working

You don't need to add fast_image.

jayvdb · 2017-01-11T02:51:29Z

bears/general/ImageDimensionBear.py

+                           'height': int(each[2])}]
+                yaml.dump(config, yaml_file, default_flow_style=False)
+
+        cmd = 'ruby -r "img_checker.rb" -e '


no need for double quotes, as there are no space characters in the filename.

jayvdb · 2017-01-11T02:52:45Z

bears/general/ImageDimensionBear.py

+
+    AUTHORS = {'The coala developers'}
+    AUTHORS_EMAILS = {'[email protected]'}
+    REQUIREMENTS = {GemRequirement(' img_checker'),


This workaround is OK for a WIP, but you need to solve the bug in GemRequirement before this bear will be merge-able.

ok, I have submitted a PR for that https://gitlab.com/coala/package_manager/merge_requests/10

robbyoconnor · 2017-01-11T05:20:18Z

FYI I made changes in abishekvashok/img_checker#22 -- soon as @Abhi2424shek merges and it deploys.

It added an executable. so you type img_checker and it will just work(tm).

robbyoconnor

Good work -- a few things not needed. See comments.

robbyoconnor · 2017-01-11T05:28:02Z

Gemfile

@@ -7,3 +7,5 @@ gem 'scss_lint', require: false# require flag is necessary https://github.com/br
 gem "reek"
 gem "puppet-lint"
 gem "csvlint"
+gem "img_checker"
+gem "fastimage"


You don't need to add fast_image.

robbyoconnor · 2017-01-11T05:29:55Z

bears/general/ImageDimensionBear.py

+                yaml.dump(config, yaml_file, default_flow_style=False)
+
+        cmd = 'ruby -r "img_checker.rb" -e '
+        cmd += '"ImgChecker.new.initialize img_config.yml"'


Not needed since soon as abishekvashok/img_checker#22 is merged -- it should "just work"

is that also updated in the gem? it would be easier if so. will do the changes once its merged. thanks for informing :)

I didn't up the version number, that's for them to decide. I just cleaned things up.

gitmate-bot · 2017-01-12T01:07:41Z

bears/general/ImageDimensionBear.py

+
+        with open('img_config.yml', 'w') as yaml_file:
+            config = [{'directory': image_file,
+                           'width': width,


E127 continuation line over-indented for visual indent'

PycodestyleBear (E127), severity NORMAL, section autopep8.

gitmate-bot · 2017-01-12T01:07:42Z

bears/general/ImageDimensionBear.py

+
+        with open('img_config.yml', 'w') as yaml_file:
+            config = [{'directory': image_file,
+                           'width': width,


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/bears/general/ImageDimensionBear.py +++ b/bears/general/ImageDimensionBear.py @@ -30,8 +30,8 @@ with open('img_config.yml', 'w') as yaml_file: config = [{'directory': image_file, - 'width': width, - 'height': height}] + 'width': width, + 'height': height}] yaml.dump(config, yaml_file, default_flow_style=False) cmd = 'img_checker' output = run(cmd, stdout=Capture(), stderr=Capture())

gitmate-bot · 2017-01-12T03:31:18Z

tests/general/ImageDimensionBearTest.py

+            shutil.which = lambda *args, **kwargs: 'path/to/git'
+            self.assertTrue(ImageDimensionBear.check_prerequisites())
+        finally:
+            shutil.which = _shutil_which    


W291 trailing whitespace'

PycodestyleBear (W291), severity NORMAL, section autopep8.

gitmate-bot · 2017-01-12T03:31:18Z

tests/general/ImageDimensionBearTest.py

+            shutil.which = lambda *args, **kwargs: 'path/to/git'
+            self.assertTrue(ImageDimensionBear.check_prerequisites())
+        finally:
+            shutil.which = _shutil_which    


The code does not comply to PEP8.

PEP8Bear, severity NORMAL, section autopep8.

The issue can be fixed by applying the following patch:

--- a/tests/general/ImageDimensionBearTest.py +++ b/tests/general/ImageDimensionBearTest.py @@ -27,7 +27,7 @@ shutil.which = lambda *args, **kwargs: 'path/to/git' self.assertTrue(ImageDimensionBear.check_prerequisites()) finally: - shutil.which = _shutil_which + shutil.which = _shutil_which def test_run_with_png(self): message = "message='The image ./test-img/img.png is larger"

gitmate-bot · 2017-01-12T03:31:19Z

tests/general/ImageDimensionBearTest.py

+            shutil.which = lambda *args, **kwargs: 'path/to/git'
+            self.assertTrue(ImageDimensionBear.check_prerequisites())
+        finally:
+            shutil.which = _shutil_which    


Line contains following spacing inconsistencies:

Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section python.

The issue can be fixed by applying the following patch:

--- a/tests/general/ImageDimensionBearTest.py +++ b/tests/general/ImageDimensionBearTest.py @@ -27,7 +27,7 @@ shutil.which = lambda *args, **kwargs: 'path/to/git' self.assertTrue(ImageDimensionBear.check_prerequisites()) finally: - shutil.which = _shutil_which + shutil.which = _shutil_which def test_run_with_png(self): message = "message='The image ./test-img/img.png is larger"

rhemon · 2017-01-12T10:31:22Z

@sils @jayvdb all test except the appveyor one (test in windows) didn't pass. my guess is that this is happening because of the space that was missing in GemRequirement in dependency_management, but the fix is done and the PR was merged https://gitlab.com/coala/package_manager/merge_requests/10/diffs . now it only needs to have a release. I hope that will have that test pass as well.

robbyoconnor

Why not document it as well?

rhemon · 2017-01-12T12:09:17Z

the documentation goes to bear-docs right? or did you mean to document in the code, how other bears had for params and returns? @robbyoconnor , if its to document code ok I am doing it now, and if its the first one, can I do it once this PR gets merged? since more changes might be requrested it gets merged.

gitmate-bot · 2017-04-16T23:46:48Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-23T22:27:10Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-23T22:27:10Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-04-30T21:06:56Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-07T19:47:21Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-20T13:56:53Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-05-27T12:36:51Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-03T11:16:48Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-10T09:57:07Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

jayvdb · 2017-06-13T12:52:34Z

coala/coala#3560 is merged. This bear can now proceed.

yukiisbored · 2017-06-14T03:19:17Z

Please make sure the images used for testing aren't copyrighted. I recommend creating your own images, I think a blank image is enough for this bear.

gitmate-bot · 2017-06-14T11:55:38Z

tests/general/ImageDimensionBearTest.py

+import shutil
+
+from bears.general.ImageDimensionBear import *
+from coalib.output.printers.LOG_LEVEL import LOG_LEVEL


This file contains unused source code.

PyUnusedCodeBear, severity NORMAL, section flakes.

The issue can be fixed by applying the following patch:

--- a/tests/general/ImageDimensionBearTest.py +++ b/tests/general/ImageDimensionBearTest.py @@ -4,9 +4,7 @@ import shutil from bears.general.ImageDimensionBear import * -from coalib.output.printers.LOG_LEVEL import LOG_LEVEL from coalib.settings.Section import Section -from coalib.settings.Setting import Setting from coalib.testing.BearTestHelper import generate_skip_decorator

gitmate-bot · 2017-06-14T11:55:39Z

tests/general/ImageDimensionBearTest.py

@@ -0,0 +1,91 @@
+import unittest
+from unittest.case import skip, skipIf


This file contains unused source code.

PyUnusedCodeBear, severity NORMAL, section flakes.

The issue can be fixed by applying the following patch:

--- a/tests/general/ImageDimensionBearTest.py +++ b/tests/general/ImageDimensionBearTest.py @@ -1,5 +1,4 @@ import unittest -from unittest.case import skip, skipIf from queue import Queue import shutil

Added ImageDimensionBear.py in bears/general, that runs the gem img_checker to check if images follow given dimensions by the user. Also added unittest for the bear, ImageDimensionBearTest.py in tests/general Closes coala#1260

gitmate-bot · 2017-06-23T09:03:56Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-30T07:43:50Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-06-30T07:43:50Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-07T06:23:46Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-14T05:03:45Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot · 2017-07-22T01:57:25Z

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

gitmate-bot added process/wip size/L labels Jan 10, 2017

rhemon force-pushed the img-dimension-bear branch from 0498502 to d6c7738 Compare January 11, 2017 01:34

jayvdb mentioned this pull request Jan 11, 2017

ImageDimensionBear #1260

Open

rhemon force-pushed the img-dimension-bear branch from d6c7738 to 31d4c0e Compare January 11, 2017 01:48

gitmate-bot added the review carefully! label Jan 11, 2017

rhemon force-pushed the img-dimension-bear branch 5 times, most recently from 459f081 to 40e5886 Compare January 11, 2017 02:47

jayvdb reviewed Jan 11, 2017

View reviewed changes

jayvdb requested changes Jan 11, 2017

View reviewed changes

robbyoconnor suggested changes Jan 11, 2017

View reviewed changes

rhemon force-pushed the img-dimension-bear branch from 40e5886 to e0d6747 Compare January 12, 2017 01:06

gitmate-bot reviewed Jan 12, 2017

View reviewed changes

rhemon force-pushed the img-dimension-bear branch 2 times, most recently from e94a8ad to 7e42486 Compare January 12, 2017 03:30

gitmate-bot reviewed Jan 12, 2017

View reviewed changes

rhemon force-pushed the img-dimension-bear branch from 7e42486 to 06c6fc5 Compare January 12, 2017 03:34

robbyoconnor suggested changes Jan 12, 2017

View reviewed changes

jayvdb added the status/blocked label Apr 9, 2017

jayvdb mentioned this pull request May 13, 2017

Allow Bears to use raw files coala/coala#3560

Merged

6 tasks

jayvdb removed the status/blocked label Jun 13, 2017

rhemon force-pushed the img-dimension-bear branch from 5a06853 to 8fa3242 Compare June 14, 2017 11:54

gitmate-bot reviewed Jun 14, 2017

View reviewed changes

rhemon force-pushed the img-dimension-bear branch 3 times, most recently from 192e032 to a7bf6c3 Compare June 15, 2017 17:21

ImageDimensionBear: Add image dimension bear

3123b7f

Added ImageDimensionBear.py in bears/general, that runs the gem img_checker to check if images follow given dimensions by the user. Also added unittest for the bear, ImageDimensionBearTest.py in tests/general Closes coala#1260

rhemon force-pushed the img-dimension-bear branch from a7bf6c3 to 3123b7f Compare June 16, 2017 05:19

rhemon mentioned this pull request Jul 3, 2017

Install Ruby gems in AppVeyor #1312

Merged

gitmate-bot added the status/STALE label Aug 31, 2017

		@@ -0,0 +1,91 @@
		import unittest
		from unittest.case import skip, skipIf

WIP ImageDimensionBear: Add ImageDimensionBear to coala-bears #1288

Are you sure you want to change the base?

WIP ImageDimensionBear: Add ImageDimensionBear to coala-bears #1288

Conversation

rhemon commented Jan 10, 2017 • edited by jayvdb Loading

gitmate-bot commented Jan 10, 2017

gitmate-bot commented Jan 10, 2017

jayvdb commented Jan 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robbyoconnor commented Jan 11, 2017

robbyoconnor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhemon commented Jan 12, 2017 • edited Loading

robbyoconnor left a comment

Choose a reason for hiding this comment

rhemon commented Jan 12, 2017 • edited Loading

gitmate-bot commented Apr 16, 2017

gitmate-bot commented Apr 23, 2017

gitmate-bot commented Apr 23, 2017

gitmate-bot commented Apr 30, 2017

gitmate-bot commented May 7, 2017

gitmate-bot commented May 20, 2017

gitmate-bot commented May 27, 2017

gitmate-bot commented Jun 3, 2017

gitmate-bot commented Jun 10, 2017

jayvdb commented Jun 13, 2017

yukiisbored commented Jun 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitmate-bot commented Jun 23, 2017

gitmate-bot commented Jun 30, 2017

gitmate-bot commented Jun 30, 2017

gitmate-bot commented Jul 7, 2017

gitmate-bot commented Jul 14, 2017

gitmate-bot commented Jul 22, 2017

rhemon commented Jan 10, 2017 •

edited by jayvdb

Loading

rhemon commented Jan 12, 2017 •

edited

Loading

rhemon commented Jan 12, 2017 •

edited

Loading

yukiisbored commented Jun 14, 2017 •

edited

Loading