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

Deprecate a number of inplace methods #652

Merged
merged 9 commits into from Nov 25, 2015

Conversation

Projects
None yet
2 participants
@jabooth
Member

jabooth commented Nov 25, 2015

We've steadily been moving away from mutating our objects in place, prefering to, where possible, return new objects with the desired state:

a = ...
a.foo()
# a is changed!
a = ...
b = a.foo()
# a is unchanged!

We try and mark mutating operations with having inplace in the method name. This PR deprecates a number of these:

  • apply_inplace()*
  • from_vector_inplace()*
  • normalize_std_inplace()
  • normalize_norm_inplace()

* in these cases the inplace variant still exists, but is now a private (_-prepended) method. We still use these methods in performance critical sections of menpo, but we are now strongly discouraging others from using them.

This PR should be brought in after #651.

@jabooth jabooth added the in progress label Nov 25, 2015

@jabooth

This comment has been minimized.

Member

jabooth commented Nov 25, 2015

@menpobot test this please

@@ -90,6 +87,23 @@ def _as_vector(self, **kwargs):
def from_vector_inplace(self, vector):
"""
Deprecated. Use hte non-mutating API, :map:`from_vector`.

This comment has been minimized.

@patricksnape
Deprecated. Use hte non-mutating API, :map:`from_vector`.
For internal usage in performance-sensitive spots,
use, see `_from_vector_inplace()`

This comment has been minimized.

@patricksnape

patricksnape Nov 25, 2015

Contributor

,use, see

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Nov 25, 2015

+1 looks good, just small typos.

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Nov 25, 2015

Happy once Appveyor is done +1

jabooth added a commit that referenced this pull request Nov 25, 2015

Merge pull request #652 from jabooth/dep_inplace
Deprecate a number of inplace methods

@jabooth jabooth merged commit 0ebd795 into menpo:master Nov 25, 2015

4 checks passed

OS X MenpoBot Jenkins build passed No test results found.
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jabooth jabooth removed the in progress label Nov 25, 2015

@jabooth jabooth deleted the jabooth:dep_inplace branch Nov 25, 2015

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