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

[query] Deprecate default_reference parameter to hl.init #13987

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions hail/python/hail/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pyspark import SparkContext

import hail
from hail.genetics.reference_genome import ReferenceGenome
from hail.genetics.reference_genome import ReferenceGenome, reference_genome_type
from hail.typecheck import (nullable, typecheck, typecheck_method, enumeration, dictof, oneof,
sized_tupleof, sequenceof)
from hail.utils import get_env_or_default
Expand Down Expand Up @@ -141,7 +141,7 @@ def default_reference(self) -> ReferenceGenome:
return self._default_ref

@default_reference.setter
def set_default_reference(self, value):
def default_reference(self, value):
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, I would not have expected this to work (two methods with the same name), but indeed it works.

In [1]: class Foo:
   ...:     def __init__(self):
   ...:         self._x = 3
   ...:     @property
   ...:     def x(self):
   ...:         return self._x
   ...:     @x.setter
   ...:     def x(self, new_x):
   ...:         self._x = new_x
   ...: 

In [2]: x = Foo()

In [3]: x.x = 3

In [4]: x.x
Out[4]: 3

In [5]: x.x = 10

In [6]: x.x
Out[6]: 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In fact, they have to have the same name. It doesn't work otherwise.

if not isinstance(value, ReferenceGenome):
raise TypeError(f'{value} is {type(value)} not a ReferenceGenome')
self._default_ref = value
Expand All @@ -166,7 +166,7 @@ def stop(self):
min_block_size=int,
branching_factor=int,
tmp_dir=nullable(str),
default_reference=enumeration(*BUILTIN_REFERENCES),
default_reference=nullable(enumeration(*BUILTIN_REFERENCES)),
idempotent=bool,
global_seed=nullable(int),
spark_conf=nullable(dictof(str, str)),
Expand All @@ -191,7 +191,7 @@ def init(sc=None,
min_block_size=0,
branching_factor=50,
tmp_dir=None,
default_reference='GRCh37',
default_reference=None,
idempotent=False,
global_seed=None,
spark_conf=None,
Expand All @@ -211,11 +211,11 @@ def init(sc=None,

This function will be called with default arguments if any Hail functionality is used. If you
need custom configuration, you must explicitly call this function before using Hail. For
example, to set the default reference genome to GRCh38, import Hail and immediately call
example, to set the global random seed to 0, import Hail and immediately call
:func:`.init`:

>>> import hail as hl
>>> hl.init(default_reference='GRCh38') # doctest: +SKIP
>>> hl.init(global_seed=0) # doctest: +SKIP

Hail has two backends, ``spark`` and ``batch``. Hail selects a backend by consulting, in order,
these configuration locations:
Expand Down Expand Up @@ -288,6 +288,8 @@ def init(sc=None,
Networked temporary directory. Must be a network-visible file
path. Defaults to /tmp in the default scheme.
default_reference : :class:`str`
*Deprecated*. Please use :func:`.default_reference` to set the default reference genome

Default reference genome. Either ``'GRCh37'``, ``'GRCh38'``,
``'GRCm38'``, or ``'CanFam3'``.
idempotent : :obj:`bool`
Expand Down Expand Up @@ -333,6 +335,14 @@ def init(sc=None,
warning('Hail has already been initialized. If this call was intended to change configuration,'
' close the session with hl.stop() first.')

if default_reference is not None:
warnings.warn('Using hl.init with a default_reference argument is deprecated. '
'To set a default reference genome after initializing hail, '
'call `default_reference` with an argument to set the '
chrisvittal marked this conversation as resolved.
Show resolved Hide resolved
'default reference genome.')
else:
default_reference = 'GRCh37'

backend = choose_backend(backend)

if backend == 'service':
Expand Down Expand Up @@ -793,7 +803,8 @@ async def _async_current_backend() -> Backend:
return (await Env._async_hc())._backend


def default_reference(new_default_reference: Optional[ReferenceGenome] = None) -> Optional[ReferenceGenome]:
@typecheck(new_default_reference=nullable(reference_genome_type))
def default_reference(new_default_reference=None) -> Optional[ReferenceGenome]:
"""With no argument, returns the default reference genome (``'GRCh37'`` by default).
With an argument, sets the default reference genome to the argument.

Expand Down