Skip to content

FIX: Replace deprecated HasTraits.get with trait_get #2534

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

Merged
merged 5 commits into from
Apr 17, 2018

Conversation

oesteban
Copy link
Contributor

Deprecation mark: https://github.com/enthought/traits/blob/a99b3f64d50c5f7f28ffc01bf69419b061f9e976/traits/has_traits.py#L1481

Fixes the following deprecation warning:

nipype/nipype/interfaces/base/specs.py:160: DeprecationWarning: use "HasTraits.trait_get" instead
  out = super(BaseTraitedSpec, self).get(**kwargs)

This warning has started to appear recently, traits must have gotten a major update.

Deprecation mark: https://github.com/enthought/traits/blob/a99b3f64d50c5f7f28ffc01bf69419b061f9e976/traits/has_traits.py#L1481

Fixes the following deprecation warning:

```
nipype/nipype/interfaces/base/specs.py:160: DeprecationWarning: use "HasTraits.trait_get" instead
  out = super(BaseTraitedSpec, self).get(**kwargs)
```

This warning has started to appear recently, traits must have gotten a major update.
@effigies
Copy link
Member

HasTraits.trait_get has been around since at least traits 3.0, so I don't think we need to check for an AttributeError here.

On the other hand, I do think we should conform to the API and move to trait_get(), ourselves. Maybe just make a copy with trait_get, so that get() will still raise DeprecationWarnings? Alternatively, we could have both call trait_get, and raise our own deprecation warnings, so that our timeline isn't dictated by traits'.

@oesteban
Copy link
Contributor Author

Instead of removing the AttributeError check, I could raise a Warning asking the user to update traits. Even though HasTraits.trait_get has been there for long, I started to see these warnings only recently, so I guess the deprecation was included later (?).

Regarding conforming our API to the traits', I'm not sure we want to change that. Nipype usually prefers the properties to access the inputs/outputs so no need for getter/setters. WDYT @satra?

@oesteban
Copy link
Contributor Author

Sorry, ref. the first comment: the idea would be removing the except AttributeError completely in nipype 2.0.

@effigies
Copy link
Member

Our minimum version is 4.6. There is no danger of someone hitting the AttributeError.

Regarding conforming our API to the traits', I'm not sure we want to change that. Nipype usually prefers the properties to access the inputs/outputs so no need for getter/setters.

Something like this is a pretty common idiom in nipype:

def _list_outputs(self):
    outputs = self._outputs().get()
    ...

@satra
Copy link
Member

satra commented Apr 15, 2018

i think we could conform to trait_get but continue supporting get in the 1.x series.

@oesteban oesteban merged commit 4cd2e42 into nipy:master Apr 17, 2018
@oesteban oesteban deleted the fix/deprecation-warning-trait_set branch April 17, 2018 23:01
@effigies effigies added this to the 1.0.3 milestone Apr 24, 2018
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.

3 participants