-
Notifications
You must be signed in to change notification settings - Fork 0
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
add function that unhardcodes features and filters #46
Conversation
Click here to view all benchmarks. |
src/resspect/fit_lightcurves.py
Outdated
def fit(data_dic: dict, output_features_file: str, | ||
number_of_processors: int = 1, | ||
feature_extractor: str = 'bazin', filters: list = ['SNPCC'], | ||
features: list = None, type: str = 'unspecified', Ia_code: list = [10]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should type
default to None
?
List of Ia codes to be used. Default is 10 from ELAsTiCC. | ||
|
||
""" | ||
if feature_extractor == 'bazin': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might combine this if/else with the one on line 373.
header = make_features_header(filters, features) | ||
|
||
multi_process = multiprocessing.Pool(number_of_processors) | ||
if feature_extractor != None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will feature_extractor
ever be None
? Would it make sense to do this check earlier in the function and raise an exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good to me. Just a couple of comments, but nothing blocking.
Working to unhardcode features and filters. This may need to be workshopped down the line but this does what we need in the meantime basically combining
fit_snpcc
fit_plasticc
andfit_TOM
. Allows any filters to be used, may need to workshop the any features functionality.