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.

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

Issue 443 uniform partition #444

Merged
merged 7 commits into from Jun 10, 2016
Merged

Issue 443 uniform partition #444

merged 7 commits into from Jun 10, 2016

Conversation

kohr-h
Copy link
Member

@kohr-h kohr-h commented Jun 9, 2016

Implements uniform_partition_options as mentioned in #443. The name is sub-optimal but quite clearly, the function should be separate from uniform_partition.

Closes #443

>>> part.cell_boundary_vecs
(array([ 0. , 0.5, 1. ]), array([ 1.5 , 1.75, 2. ]))
"""
# np.size(None) = 1
Copy link
Member Author

Choose a reason for hiding this comment

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

nitpicking: # np.size(None) == 1

@adler-j
Copy link
Member

adler-j commented Jun 10, 2016

Functionality i great, but I'm not sure that this should be separate from uniform_partition, there is a degree of clutter to this.

Regarding tol there is a leftover in contains_set, and contains_all seems to be lacking the parameter fully. Might as well fix while we're at it.

@kohr-h kohr-h force-pushed the issue-443__uniform_partition branch from 17118d9 to 1f6a7ee Compare June 10, 2016 10:42
@kohr-h
Copy link
Member Author

kohr-h commented Jun 10, 2016

Done with all the comments and rebased. Waiting for Travis to be happy.

Now the functionality is merged into uniform_partition.

@kohr-h kohr-h added the merge? label Jun 10, 2016
cell_sides=None, nodes_on_bdry=False):
"""Return a partition with equally sized cells.

This function offers more flexibility over `uniform_partition` in
Copy link
Member

@adler-j adler-j Jun 10, 2016

Choose a reason for hiding this comment

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

Old comment. Should be updated with call syntax.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops

@adler-j
Copy link
Member

adler-j commented Jun 10, 2016

Great changes, some minor comments then we should be fine.

@adler-j adler-j closed this Jun 10, 2016
@adler-j adler-j reopened this Jun 10, 2016
@adler-j
Copy link
Member

adler-j commented Jun 10, 2016

Missclick on close...

Instead of always expecting begin, end and num_nodes, now
cell_sides can be given in addition, and one of the first
three parameters can be left out instead. This can be done
per axis.
@kohr-h kohr-h force-pushed the issue-443__uniform_partition branch from 1f6a7ee to 024552f Compare June 10, 2016 11:47
@kohr-h
Copy link
Member Author

kohr-h commented Jun 10, 2016

Fixed the old stuff, will merge after tests have run.

@adler-j adler-j added merge! and removed merge? labels Jun 10, 2016
@kohr-h kohr-h merged commit ecfd306 into master Jun 10, 2016
@kohr-h kohr-h deleted the issue-443__uniform_partition branch June 10, 2016 11:56
@kohr-h kohr-h removed the merge! label Jun 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants