Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

world.plates->numpy, some optimizations #201

Merged
merged 2 commits into from
Nov 18, 2015
Merged

world.plates->numpy, some optimizations #201

merged 2 commits into from
Nov 18, 2015

Conversation

tcld
Copy link
Contributor

@tcld tcld commented Nov 17, 2015

It seems I overlooked an array when making the other arrays numpy-arrays.
I stumbled over this after noticing that center_world(), for some reason, returned different results for Python 2 and 3 when I generated the following test-world. That is fixed with this, too.

width=1024
height=768
plates=3
ocean=0.71
seed=13051

Short summary of the changes in here:

  • Overlooked world.plates when switching to numpy.
  • Added world.plates to the serialization-tests (It had been stored before, but never was tested, I think.)
  • Reworked center_land() to make use of numpy.
  • Removed some superfluous functions/tests.

PS: Sorry for still doing stuff that is not related to an immediate release!

@tcld
Overlooked world.plates when switching to numpy.
1272249 
Added world.plates to the serialization-tests.
Reworked center_land().
@psi29a
Copy link
Member

psi29a commented Nov 17, 2015

This actually looks like it should be included, because it falls under the mandate of 'use numpy, everywhere!'.

@ftomassetti wouldn't you agree?

ftomassetti
ftomassetti reviewed Nov 17, 2015
View reviewed changes
tests/common_test.py
@@ -16,19 +16,6 @@ def test_get_and_set_verbose(self):
set_verbose(False)
self.assertEqual(False, get_verbose())

def test_matrix_min_and_max(self):
Copy link
Member

Choose a reason for hiding this comment

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

Why we cancel the test but not the tested function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may have forgotten, will check that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code removed.

@ftomassetti
Copy link
Member

@psi29a I have nothing against it and the code seems ok to me, feel free to merge it!

tcld
tcld reviewed Nov 18, 2015
View reviewed changes
tests/common_test.py
@@ -40,25 +26,6 @@ def test_counter(self):
c.count("a")
self.assertEqual("a : 1\nb : 3\n", c.to_str())

def test_rescale_value(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use numpy.interp() if something like rescale_value() is needed. As with other functions in common.py, it wasn't used anywhere anyway.

@tcld
Copy link
Contributor Author

tcld commented Nov 18, 2015

Comments taken care of. I also removed another test + function since it wasn't used anywhere (and, like matrix_min_and_max(), had a numpy counterpart).

@ftomassetti
Copy link
Member

Let's merge #198 and then this one before changing release number and tagging the new release

ftomassetti added a commit that referenced this pull request Nov 18, 2015
@ftomassetti
Merge pull request #201 from tcld/numpy_plates
3e8030e 
world.plates->numpy, some optimizations
@ftomassetti ftomassetti merged commit 3e8030e into Mindwerks:master Nov 18, 2015
@tcld tcld deleted the numpy_plates branch November 18, 2015 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tcld @psi29a @ftomassetti