Fix block.py and add test for file block.py #25

hashbangstudio · 2014-10-01T17:47:17Z

For Block class:
Add docstrings for functions in block.py
Fix references to attribute so all self.type not split between self.type and self.id
Add id property to maintain backwards compatibility
type and data are always integers and should be treated as such in __repr__
As __cmp__ no longer in Python 3 need to use functools.total_ordering which is in 2.7 and 3.*
https://docs.python.org/2/library/functools.html#functools.total_ordering
https://docs.python.org/3/library/functools.html#functools.total_ordering
Define __eq__ and __lt__ and others are created automatically

For blockType enumeration:
Move to back ported IntEnum as allows direct integer comparison
https://pypi.python.org/pypi/enum34
https://docs.python.org/3/library/enum.html#intenum

Add test_block.py:
A unit test module to test the block file

For Block class: Add docstrings for functions in block.py Fix references to attribute so all self.type not split between self.type and self.id Add id property to maintain backwards compatibility type and data are always integers and should be treated as such in __repr__ As __cmp__ no longer in Python 3 need to use functools.total_ordering which is in 2.7 and 3.* https://docs.python.org/2/library/functools.html#functools.total_ordering https://docs.python.org/3/library/functools.html#functools.total_ordering Define __eq__ and __lt__ and others are created automatically For blockType enumeration: Move to back ported IntEnum as allows direct integer comparison https://pypi.python.org/pypi/enum34 https://docs.python.org/3/library/enum.html#intenum Add test_block.py: A unit test module to test the block file

doismellburning · 2014-10-01T18:08:56Z

Why are blocks orderable? Does some odd bit of the API depend on this? :s

hashbangstudio · 2014-10-01T18:11:30Z

Don't have to orderable as far as I am aware but they had been defined as such with __cmp__ in the original API and thought safest to replicate the original behaviour. Could just define __eq__ and __ne__ if people prefer.

hashbangstudio · 2014-10-01T18:12:52Z

Until enum34 package defined in Travis, build will fail see #23 as enum won't exist.

doismellburning · 2014-10-01T18:13:24Z

I don't think nonsensical orderings are helpful - will look to see if it's actually used anywhere (I hope not...)

doismellburning · 2014-10-01T18:24:46Z

tests/test_block.py

+        for index, attr in enumerate(b):
+            self.assertEqual(attr, idAndData[index])
+
+    def testBlockConstants(self):


I may be alone here, but I'm unconvinced by the value of testing constants - persuade me...?

hashbangstudio · 2014-10-01T18:40:03Z

Testing that the name exists as well as the value in this case with enum.
Thinking was in case a line in the blockType definition gets deleted/altered, or name has been changed accidentally from e.g. global find and replace.
In both cases enum is still valid to interpreter, but one element of enum that would be used in code won't exist.
Should always be caught on commit but have seen mistakes happen or all modifications committed unintentionally.
Additionally are checking the aliasing of water and lava to flowing variants.
Easy enough to lose if unwanted.

This reverts commit ff8a464. Oops, it's a python package not a deb!

dbrgn · 2014-10-01T20:07:41Z

minecraft/block.py

+        block.data = The variant of the type of block.
+        For example the colour of wool or the orientation of stairs
+        The default type for blocks is dirt
+        """


Why the indentation change?

Yes, this is unnecessary, can you keep the indentation of docstrings to one level in line with PEP 257 please.

@ghickman Got the pep8/flake8 skillz to add this to tests so you don't need to keep hand-PEPing? ;P

unintentional will alter

@hashbangstudio – Thanks!

This should hopefully mean that build will pass with enum dependency met

doismellburning · 2014-10-01T22:01:00Z

minecraft/block.py

+    def __eq__(self, rhs):
+        """
+            Equality override
+            Two blocks are equal only if their hashes are equal


Except that's only true in this case where hash doesn't collide, and not generally true - would you mind making a proper __eq__ please?

Fix __eq__ so block only equal if all attributes equal Fix apostrophe usage

Move to PEP8 lowercase_name method names Move test of water and lava aliases out to separate test

Add function (self): that was left missing blockType changed to BlockType

Change functions and variables to lower_case

doismellburning · 2014-10-02T08:08:36Z

minecraft/block.py



-class blockType(Enum):


I think this changes a bit of the API we don't want to change, but we aren't testing thoroughly enough :s

hashbangstudio · 2014-10-02T11:44:03Z

It will need to be changed for PEP8 naming and can't see where it is used in the rest of the API at the moment, unless I am just not seeing it. Can drop it from PR if it is used somewhere.

Remove unittest style file Add py.test style file

Variable missed in conversion to PEP8 standards Change variable name to avoid confusion between withData(data) and the Block(type, data) created instances

Add comparison test using eval(repr()) Change to comparing against a constant string

doismellburning · 2014-10-15T20:48:12Z

So, sorry about this, but I had to revert a bunch of other changes to fix compatibility breakages they introduced. Are you ok to fix the merge conflicts here, or would you like a hand? Cheers, apologies!

hashbangstudio · 2014-10-17T13:34:31Z

Will have a look to see if can fix conflicts over the weekend, if not will let you know

doismellburning · 2014-10-17T23:25:14Z

Awesome, thanks, sorry again!

both added test_block.py. Combined in one file

Add BlockType enum back in Maintain backwards compatibility

doismellburning · 2014-10-19T09:51:26Z

<3 thanks for the fixes!

doismellburning · 2014-10-19T09:52:52Z

mcpi/block.py

+    For example grass, sand, dirt or wool
+    block.data = The variant of the type of block.
+    For example the colour of wool or the orientation of stairs
+    The default type for blocks is dirt


Maybe I'm a bit too keen on building obsidian houses, but I'm personally very wary about introducing defaults for what is a somewhat arbitrary choice...

remove docstring comment on default type

hashbangstudio · 2014-10-19T10:40:39Z

Agree is arbitrary, was left in from previous API refactoring. Have removed default assignment.

doismellburning reviewed Oct 1, 2014
View reviewed changes

doismellburning and others added 6 commits October 1, 2014 20:03

Added missing enum34 dependency

ff8a464

Rename test to indicate not py.test form

4d18907

Revert "Added missing enum34 dependency"

d6db56f

This reverts commit ff8a464. Oops, it's a python package not a deb!

Remove runner code in test

9a1edc7

Remove block ordering comparisons

b69f607

Remove ordered comparison tests

9182af3

dbrgn reviewed Oct 1, 2014
View reviewed changes

hashbangstudio added 2 commits October 1, 2014 21:26

Undo unintentional indent change

1c6ec04

Update setup.py to inline with master

810f99d

This should hopefully mean that build will pass with enum dependency met

doismellburning reviewed Oct 1, 2014
View reviewed changes

hashbangstudio added 2 commits October 1, 2014 23:23

Fix comparison and typo

728b943

Fix __eq__ so block only equal if all attributes equal Fix apostrophe usage

Use PEP8 method names and clarify enum test

acf013c

Move to PEP8 lowercase_name method names Move test of water and lava aliases out to separate test

hashbangstudio mentioned this pull request Oct 1, 2014

Add blockData Enums #27

Open

hashbangstudio added 3 commits October 2, 2014 00:09

Fix missing func def and class naming

bed58c6

Add function (self): that was left missing blockType changed to BlockType

Hopefully correct unintentional indent change

11bb748

Move to PEP8 compliant naming

c8dad24

Change functions and variables to lower_case

doismellburning reviewed Oct 2, 2014
View reviewed changes

hashbangstudio added 3 commits October 2, 2014 14:00

Move to py.test style

2d3500c

Remove unittest style file Add py.test style file

Rename for clarity and PEP8

3c1fd01

Variable missed in conversion to PEP8 standards Change variable name to avoid confusion between withData(data) and the Block(type, data) created instances

Add eval repr test and use constant string

f42e1f3

Add comparison test using eval(repr()) Change to comparing against a constant string

hashbangstudio added 4 commits October 19, 2014 06:10

Merge remote-tracking branch 'upstream/block-update' into block-update

ffb7ca3

Merge remote-tracking branch 'upstream/master' into block-update

b677dc6

Fix conflict between files

a3e3a00

both added test_block.py. Combined in one file

Add new block elements

8cb35c0

Add BlockType enum back in Maintain backwards compatibility

doismellburning reviewed Oct 19, 2014
View reviewed changes

hashbangstudio added 2 commits October 19, 2014 11:37

Remove default type assignment

23f43fe

Also remove the comment line

2f58179

remove docstring comment on default type

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix block.py and add test for file block.py #25

Fix block.py and add test for file block.py #25

hashbangstudio commented Oct 1, 2014

doismellburning commented Oct 1, 2014

hashbangstudio commented Oct 1, 2014

hashbangstudio commented Oct 1, 2014

doismellburning commented Oct 1, 2014

doismellburning Oct 1, 2014

hashbangstudio commented Oct 1, 2014

dbrgn Oct 1, 2014

ghickman Oct 1, 2014

doismellburning Oct 1, 2014

hashbangstudio Oct 1, 2014

ghickman Oct 1, 2014

doismellburning Oct 1, 2014

doismellburning Oct 2, 2014

hashbangstudio commented Oct 2, 2014

doismellburning commented Oct 15, 2014

hashbangstudio commented Oct 17, 2014

doismellburning commented Oct 17, 2014

doismellburning commented Oct 19, 2014

doismellburning Oct 19, 2014

hashbangstudio commented Oct 19, 2014

Fix block.py and add test for file block.py #25

Are you sure you want to change the base?

Fix block.py and add test for file block.py #25

Conversation

hashbangstudio commented Oct 1, 2014

doismellburning commented Oct 1, 2014

hashbangstudio commented Oct 1, 2014

hashbangstudio commented Oct 1, 2014

doismellburning commented Oct 1, 2014

Choose a reason for hiding this comment

hashbangstudio commented Oct 1, 2014

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

hashbangstudio commented Oct 2, 2014

doismellburning commented Oct 15, 2014

hashbangstudio commented Oct 17, 2014

doismellburning commented Oct 17, 2014

doismellburning commented Oct 19, 2014

Choose a reason for hiding this comment

hashbangstudio commented Oct 19, 2014