Ouranosinc / xclim

Library of derived climate variables, ie climate indicators, based on xarray.
https://xclim.readthedocs.io/en/stable/
Apache License 2.0
332 stars 59 forks source link

Developing new indices #212

Closed Balinus closed 5 years ago

Balinus commented 5 years ago

Description

Adding a new index in _simple.py is not propagated to the actual indices module. Reproducible steps:

  1. Add function def foo(): in _simple.py
    def foo():
    return 0
  2. Start python
  3. ìmport xclim.indices as xci
  4. Trying to access foo with xci.foo returns
Traceback (most recent call last):
  File "<input>", line 1, in <module>
AttributeError: module 'xclim.indices' has no attribute 'foo'

note that the indices is accessible bi a direct call. e.g.

xci._simple.foo()
0
huard commented 5 years ago

You have to add the function name to __name__.

tlvu commented 5 years ago

It's actually to __all__ list in _simple.py. But not sure we need __all__ in the first place.

huard commented 5 years ago

Right ! I think we need it to play nicely with the from _indices import * and avoid importing non-indices objects.

tlvu commented 5 years ago

Right, but it seems that __all__ is containing everything in _simple.py now so we are not yet leveraging it to hide any non-indices yet. By the time we have non-indices, we can simply put those non-indices in another file and not do import * of that new file so those non-indices won't be exposed.

Brief, if we get more simple support calls like this, maybe remove the usage of __all__ to not confuse future contributors.

huard commented 5 years ago

What I meant is that without all, we'd also import logging, xr, etc.

tlvu commented 5 years ago

Oh true ! I didn't realize import * will import other modules as well. We're stuck with __all__ then.

Balinus commented 5 years ago

Perfect, will try with the proposed solution. Closing this until I can work on this in my local copy. Thanks!