Comet Logging #1
Conversation
…ded conditionals in clustering.py, extended create_model in oop
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
…to comet_logging
pycaret/loggers/comet_logger.py
Outdated
| self.run.log_others(experiment_custom_tags) | ||
|
|
||
| def log_sklearn_pipeline(self, experiment, prep_pipe, model, path=None): | ||
|
|
There was a problem hiding this comment.
to remove explicit dependency on joblib it's possible to add a method to set a "model_dumper" property on this class
def __init__(self, pipline_dumper):
self._pipeline_dumper = pipeline_dumper # user can pass joblib.dump or any other suitable function the price to pay is that it's not part of the standard API
pycaret/loggers/comet_logger.py
Outdated
|
|
||
| pipeline = deepcopy(prep_pipe) | ||
| pipeline.steps.append(["trained_model", model]) | ||
| joblib.dump(pipeline, 'pipeline.pkl') |
There was a problem hiding this comment.
better use a tempfile.NamedTemporaryFile and not a hard coded 'pipeline.pkl' - it will also remove itself if used as a context manager
| self.run = None | ||
|
|
||
| def init_experiment(self, exp_name_log, full_name=None, **kwargs): | ||
| self.run = comet_ml.Experiment(project_name=exp_name_log, **kwargs) |
There was a problem hiding this comment.
is there a reason to call it "run" and not "experiment"?
does it have to be public?
There was a problem hiding this comment.
run is what they used for other loggers and they have made it public. so just am following the maintainer's format
| if "Object" in result_copy: | ||
| result_copy["Object"] = result_copy["Object"].apply( | ||
| lambda obj: str(type(obj).__name__) | ||
| ) |
There was a problem hiding this comment.
this transforamtion should be extracted to another function with a meaningful name, so we understand why it is needed
pycaret/loggers/comet_logger.py
Outdated
| def log_artifact(self, file, type="artifact"): | ||
| file_name, extension = None, "" | ||
| file_pathlib = Path(file) | ||
| file_name = file_pathlib.stem.replace(" ", "_") + str(uuid.uuid1())[:8] |
There was a problem hiding this comment.
too much going on in one line.
better do this
sanitized_step = Path(file).stem.replace(" ", "_")
short_uiud = str(uuid.uuid1())[:8]
nicer to use "{stem}_{uuid}".format(stem=sanitized_stem, short_uuid)Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
…p_to_3.0.0 Remove nightly build, bump version to 3.0.0
Added and organized badges
Pin numpy<1.24
…terror Cleanup cuML ImportError message
add duplicate indices check
added index cleaning utility for time series
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
…e_#3457 fix of CatboostRegresser on GPU with distribution.
Related Issue or bug
Info about Issue or bug
Closes #[issue number that will be closed through this PR]
Describe the changes you've made
A clear and concise description of what you have done to successfully close your assigned issue. Any new files? or anything you feel to let us know!
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Describe if there is any unusual behaviour of your code(Write
NAif there isn't)A clear and concise description of it.
Checklist:
Screenshots