-
Notifications
You must be signed in to change notification settings - Fork 57
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 options to check task definition to create an output #205
Conversation
@hiro-o918 Thanks great suggestions. Is it possible to apply a yapf formatter? |
LGTM. I'll wait for the @Hi-king review. |
@hiro-o918 |
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.
Thank you for your great contribution!
@@ -316,7 +339,7 @@ def test_add_configuration(self, mock_config: MagicMock): | |||
self.assertEqual(True, kwargs['bool_param']) | |||
|
|||
@patch('luigi.cmdline_parser.CmdlineParser.get_instance') | |||
def test_add_cofigureation_evaluation_order(self, mock_cmdline: MagicMock): | |||
def test_add_configuration_evaluation_order(self, mock_cmdline: MagicMock): |
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.
👍
@@ -369,7 +392,7 @@ def test_load_index_only_dataframe(self): | |||
task = _DummyTask() | |||
task.load = MagicMock(return_value=pd.DataFrame(index=range(3))) | |||
|
|||
# connnot load index only frame with required_columns | |||
# cannot load index only frame with required_columns |
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.
👍
Cool solution! For workaround, currently we're using |
Thank you for your fast your reviews! 🚀 |
@hiro-o918 gokart 1.0.1 including your great contribution is released. Thanks again! |
This reverts commit d5e55f4.
What
Add a feature to create a hash of a task.
The hash is created by
cloudpickle.dumps
to the class object of a task.Why
Current implementation assumes that an output is determined by input parameters and requirements.
But this assumption does not work, when the implementation of a task changes.
As a results, some issues can occur.
.pkl
to rerun tasksmodification_time_check
can resolve this in some aspects (I like this option).pkl
must be removed as well as local development.So, adding task definition to hash resolves these issues and re-reruns tasks safely.
Discussion
cloudpickle