Config can read map and assign itself a mapping function #79
Conversation
Users should not be required to apply interpolation functions
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| """ | ||
| Map is a special config which takes input file. | ||
| The method `apply` is dynamically assigned. | ||
| When using points, the `apply` will be `map_point`, |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
W291 trailing whitespace
| @@ -136,34 +143,104 @@ def build(self, llh_name: str = None): | |||
|
|
|||
| data = load_json(self.file_path) | |||
|
|
|||
| if data['coordinate_type'] == 'point': | |||
| coordinate_type = data['coordinate_type'] | |||
| if coordinate_type == 'point' or coordinate_type == 'log_point': | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS514 Found implicit in condition
| self.build_point(data) | ||
| elif data['coordinate_type'] == 'regbin': | ||
| elif coordinate_type == 'regbin' or coordinate_type == 'log_regbin': |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS514 Found implicit in condition
| self.build_regbin(data) | ||
| else: | ||
| raise ValueError("map_type must be either 'point' or 'regbin'!") | ||
|
|
||
| def build_point(self, data): | ||
| """Cache the map to jnp.array if bins_type is point""" | ||
| if data['coordinate_name'] == 'pdf' or data['coordinate_name'] == 'cdf': |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS514 Found implicit in condition
| self.build_regbin(data) | ||
| else: | ||
| raise ValueError("map_type must be either 'point' or 'regbin'!") | ||
|
|
||
| def build_point(self, data): | ||
| """Cache the map to jnp.array if bins_type is point""" | ||
| if data['coordinate_name'] == 'pdf' or data['coordinate_name'] == 'cdf': |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS204 Found overused expression: data['coordinate_name']; used 8 > 7
| ) | ||
| setattr(self, 'preprocess', self.log_pos) | ||
| else: | ||
| setattr(self, 'preprocess', self.linear_pos) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
B010 Do not call setattr with a constant attribute value, it is not any safer than normal property access.
| setattr(self, 'preprocess', self.log_pos) | ||
| else: | ||
| setattr(self, 'preprocess', self.linear_pos) | ||
| setattr(self, 'apply', self.map_regbin) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
B010 Do not call setattr with a constant attribute value, it is not any safer than normal property access.
| setattr(self, 'preprocess', self.linear_pos) | ||
| setattr(self, 'apply', self.map_regbin) | ||
|
|
||
| def map_regbin(self, pos): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
D102 Missing docstring in public method
| ) | ||
| return val | ||
|
|
||
| def linear_pos(self, pos): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
D102 Missing docstring in public method
| def linear_pos(self, pos): | ||
| return pos | ||
|
|
||
| def log_pos(self, pos): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
D102 Missing docstring in public method
Pull Request Test Coverage Report for Build 4047388884
💛 - Coveralls |
| @@ -6,6 +6,8 @@ | |||
|
|
|||
| from appletree.share import _cached_configs | |||
| from appletree.utils import exporter, load_json, get_file_path, integrate_midpoint, cum_integrate_midpoint | |||
| from appletree import interpolation | |||
| from appletree.interpolation import FLOAT_POS_MIN, FLOAT_POS_MAX | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS458 Found imports collision: appletree.interpolation
| @@ -6,6 +6,8 @@ | |||
|
|
|||
| from appletree.share import _cached_configs | |||
| from appletree.utils import exporter, load_json, get_file_path, integrate_midpoint, cum_integrate_midpoint | |||
| from appletree import interpolation | |||
| from appletree.interpolation import FLOAT_POS_MIN, FLOAT_POS_MAX | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS458 Found imports collision: appletree.interpolation
| dr2 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos2 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr3 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos3 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr1 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos1 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 23 > 14
| dr2 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos2 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr3 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos3 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr1 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos1 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
E501 line too long (109 > 100 characters)
| dr3 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos3 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=1e-10) | ||
| dr1 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos1 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr2 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos2 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 23 > 14
| dr3 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos3 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr5 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos5 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr6 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos6 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
E501 line too long (109 > 100 characters)
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr5 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos5 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr6 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos6 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr7 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos7 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 23 > 14
| dr4 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos4 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr5 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos5 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr6 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos6 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr7 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos7 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
E501 line too long (109 > 100 characters)
| dr5 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos5 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr6 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos6 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr7 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos7 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr8 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos8 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 23 > 14
| dr5 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos5 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr6 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos6 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr7 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos7 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) | ||
| dr8 = jnp.clip(jnp.sqrt(jnp.sum((ref_pos8 - pos)**2, axis=-1)), a_min=FLOAT_POS_MIN, a_max=FLOAT_POS_MAX) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
E501 line too long (109 > 100 characters)
| from . import nr_lyqy | ||
| from .nr_lyqy import * | ||
| from . import lyqy | ||
| from .lyqy import * |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS458 Found imports collision: lyqy
| def build(self, llh_name: str = None): | ||
| """Read maps""" | ||
| if self.name in _cached_configs: | ||
| _configs = _cached_configs[self.name] |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
appletree/config.py
Outdated
| """Read maps""" | ||
| if self.name in _cached_configs: | ||
| _configs = _cached_configs[self.name] | ||
| if isinstance(_configs, dict): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
appletree/config.py
Outdated
| _configs = _cached_configs[self.name] | ||
| if isinstance(_configs, dict): | ||
| try: | ||
| self._configs = _configs[llh_name] |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
appletree/config.py
Outdated
| mesg += f'but it is {llh_name}.' | ||
| raise ValueError(mesg) | ||
| else: | ||
| self._configs = _configs |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
appletree/config.py
Outdated
| _cached_configs.update({self.name: self._configs}) | ||
| self._configs_default = self.get_default() | ||
|
|
||
| self.median = Map(name=self.name + '_median', default=self._configs_default[0]) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 15 > 14
appletree/config.py
Outdated
| self.median = Map(name=self.name + '_median', default=self._configs_default[0]) | ||
| _cached_configs[self.median.name] = self._configs[0] | ||
| self.median.build() | ||
| self.lower = Map(name=self.name + '_lower', default=self._configs_default[1]) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 15 > 14
appletree/config.py
Outdated
| self.lower = Map(name=self.name + '_lower', default=self._configs_default[1]) | ||
| _cached_configs[self.lower.name] = self._configs[1] | ||
| self.lower.build() | ||
| self.upper = Map(name=self.name + '_upper', default=self._configs_default[2]) |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 15 > 14
appletree/config.py
Outdated
| self.upper.build() | ||
|
|
||
| def apply(self, pos, sigma): | ||
| """ |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
D200 One-line docstring should fit on one line with quotes
appletree/plugins/efficiency.py
Outdated
| from appletree.plugin import Plugin | ||
| from appletree.config import Map | ||
| from appletree.config import Map, SigmaMap |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
F401 'appletree.config.Map' imported but unused
|
@xzh19980906 There is a mess. But the most important file is |
|
|
||
| maps = {} | ||
| for i, sigma in enumerate(['median', 'lower', 'upper']): | ||
| maps[sigma] = Map( |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS204 Found overused expression: maps[sigma]; used 5 > 4
appletree/config.py
Outdated
| maps[sigma] = Map( | ||
| name=self.name + f'_{sigma}', | ||
| default=self._configs_default[i]) | ||
| if not maps[sigma].name in _cached_configs.keys(): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS508 Found incorrect not with compare usage
appletree/config.py
Outdated
| # Sanity check | ||
| if isinstance(self.default, dict): | ||
| raise ValueError( | ||
| f"Do not set {self.name}'s default value as dict!" |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
C812 missing trailing comma
| if not self.name in _cached_configs: | ||
| _cached_configs.update({self.name: self.get_default()}) | ||
| if self.name in _cached_configs: | ||
| value = _cached_configs[self.name] |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
appletree/config.py
Outdated
| else: | ||
| value = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = {llh_name: value} |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
appletree/config.py
Outdated
| @@ -120,7 +138,8 @@ def build(self, llh_name: str = None): | |||
| file_path = _cached_configs[self.name] | |||
| else: | |||
| file_path = get_file_path(self.get_default()) | |||
| _cached_configs.update({self.name: file_path}) | |||
| # Update values to sharing dictionary | |||
| _cached_configs[self.name] = {llh_name: file_path} | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
appletree/config.py
Outdated
| else: | ||
| self._configs = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = {llh_name: self._configs} |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = {llh_name: self._configs} | ||
|
|
||
| if isinstance(_configs, dict): |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
|
|
||
| if isinstance(_configs, dict): | ||
| try: | ||
| self._configs = _configs[llh_name] |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
| mesg += f'but it is {llh_name}.' | ||
| raise ValueError(mesg) | ||
| else: | ||
| self._configs = _configs |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
| if self.name in _cached_configs: | ||
| _configs = _cached_configs[self.name] | ||
| else: | ||
| _configs = self.get_default() |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
appletree/config.py
Outdated
| else: | ||
| _configs = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = {llh_name: _configs} |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
appletree/config.py
Outdated
| else: | ||
| _configs = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = {llh_name: _configs} |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
| else: | ||
| value = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = value |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
| @@ -120,7 +137,8 @@ def build(self, llh_name: str = None): | |||
| file_path = _cached_configs[self.name] | |||
| else: | |||
| file_path = get_file_path(self.get_default()) | |||
| _cached_configs.update({self.name: file_path}) | |||
| # Update values to sharing dictionary | |||
| _cached_configs[self.name] = file_path | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
| else: | ||
| _configs = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = _configs |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS529 Found implicit .get() dict usage
| else: | ||
| _configs = self.get_default() | ||
| # Update values to sharing dictionary | ||
| _cached_configs[self.name] = _configs |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS121 Found usage of a variable marked as unused: _configs
| @@ -48,9 +48,12 @@ def __init__(self, name: str = None, **config): | |||
| warning = f'The usage of meshgrid binning is highly discouraged.' | |||
| warn(warning) | |||
| self.component_bins_type = 'meshgrid' | |||
| x_bins = jnp.linspace(*config['x_clip'], self._bins[0] + 1) | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 15 > 14
| @@ -48,9 +48,12 @@ def __init__(self, name: str = None, **config): | |||
| warning = f'The usage of meshgrid binning is highly discouraged.' | |||
| warn(warning) | |||
| self.component_bins_type = 'meshgrid' | |||
| x_bins = jnp.linspace(*config['x_clip'], self._bins[0] + 1) | |||
| y_bins = jnp.linspace(*config['y_clip'], self._bins[1] + 1) | |||
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS221 Found line with high Jones Complexity: 15 > 14
| maps = {} | ||
| sigmas = ['median', 'lower', 'upper'] | ||
| for i, sigma in enumerate(sigmas): | ||
| maps[sigma] = Map( |
There was a problem hiding this comment.
[pep8] reported by reviewdog 🐶
WPS204 Found overused expression: maps[sigma]; used 6 > 4
Users should not be required to apply interpolation functions, so we would like to make the
Configclass more clever. The methodapplyis dynamically assigned. When using points, theapplywill bemap_point, while using regular binning, theapplywill bemap_regbin. When using log-binning, we will first convert the positions to log space.Like:
This will make the functionality of Config and Plugin completely de-coupled.
Finally, add a Config called
SigmaMapto handle the uncertainty of efficiency maps.