Skip to content
This repository has been archived by the owner on Jun 22, 2022. It is now read-only.

removed suffixes #110

Merged
merged 2 commits into from
Oct 5, 2018
Merged

removed suffixes #110

merged 2 commits into from
Oct 5, 2018

Conversation

kamil-kaczmarek
Copy link
Member

No description provided.

steppy/base.py Outdated
if self._name_unique():
_ALL_STEPS_NAMES.append(self.name)
else:
raise ValueError('Step with name "{}", already exist. You must assign unique Step name. '
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kamil-kaczmarek I think this will cause error when I do the following.

Create pipeline1 with step resize
Create independent pipeline2 with step resize

This happens when I have train() where I define pipeline and later in evaluate() I also define pipeline.

I think in order to deal with that we may need to drop _ALL_STEPS_NAMES altogether.

Copy link
Member Author

Choose a reason for hiding this comment

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

in this case we will not provide Users with any warning when they create several steps with name my_model -> new Step will simply overwrite previous one.

@kamil-kaczmarek kamil-kaczmarek self-assigned this Oct 5, 2018
… two names are the same raise an error, simplified error logs
@kamil-kaczmarek
Copy link
Member Author

@jakubczakon corrections ready

@jakubczakon jakubczakon merged commit 520a17a into master Oct 5, 2018
@jakubczakon jakubczakon deleted the dev-s12 branch October 5, 2018 14:54
@kamil-kaczmarek kamil-kaczmarek restored the dev-s12 branch October 5, 2018 14:58
kamil-kaczmarek pushed a commit that referenced this pull request Oct 5, 2018
kamil-kaczmarek pushed a commit that referenced this pull request Oct 5, 2018
@kamil-kaczmarek kamil-kaczmarek deleted the dev-s12 branch October 5, 2018 15:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants