Skip to content

WIP: Catalog handler#4

Open
aguinot wants to merge 5 commits intomainfrom
catalog_handler
Open

WIP: Catalog handler#4
aguinot wants to merge 5 commits intomainfrom
catalog_handler

Conversation

@aguinot
Copy link
Owner

@aguinot aguinot commented Jan 24, 2023

First version of the catalo handler.
See issues #3 for more details.

Right there are two child class GalaxyCatalog and StarCatalog which do not have a very good reason to exist. Will see if we keep them. It could be useful in case one of the catalog requiered a specific handling. Which is not the case at the moment..

aguinot and others added 5 commits January 22, 2023 06:00
First version of the config parser
First version of the Catalog class to deal with the input catalogs
Comment on lines +31 to +37
if isinstance(config, ConfigParser):
self._config = config.config
else:
raise ValueError(
f"config must be an instance of {ConfigParser}. "
f"Got: {type(config)}"
)

Choose a reason for hiding this comment

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

Suggested change
if isinstance(config, ConfigParser):
self._config = config.config
else:
raise ValueError(
f"config must be an instance of {ConfigParser}. "
f"Got: {type(config)}"
)
if not isinstance(config, ConfigParser):
raise ValueError(
f"config must be an instance of `WeakLensingValidation.config_parser.ConfigParser`. "
f"Got: {type(config)}"
)
self._config = config.config

def __init__(
self,
path=None,
config=None,

Choose a reason for hiding this comment

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

Pourquoi mettre par défaut à None ?
C'est un argument nécessaire, pas optionnel.

Comment on lines +80 to +81
self._config["workspace"]['path'] + '/' \
+ file_name + '.hdf5'

Choose a reason for hiding this comment

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

Use pathlib or os.path.join

)
else:
new_path = \
self._config["workspace"]['path'] + '/' + file_name

Choose a reason for hiding this comment

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

same here

Comment on lines +105 to +107
df_tmp['var_cat_history'] = \
np.ones(len(df_tmp), dtype=int) \
* path_config['var_cat_history'][i]

Choose a reason for hiding this comment

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

Avoid backslashes, use parentheses instead

return self._df[col_name].to_dask_array()
elif isinstance(col_name, dict):
cat_tmp = {}
func = copy.copy(col_name['func'])

Choose a reason for hiding this comment

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

Suggested change
func = copy.copy(col_name['func'])
func = copy.deepcopy(col_name['func'])

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.

2 participants