Skip to content

Remove check_pickle argument in delayed. #903

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

Merged
merged 4 commits into from
Nov 16, 2020

Conversation

jjerphan
Copy link
Contributor

Context: check_pickle should have been removed in 0.13.0 as indicated in the DeprecationWarning.

Changes: This PR removes this argument, behavior, and the associated test.

Associated Issue: #900

@jjerphan jjerphan closed this May 13, 2020
@pierreglaser
Copy link
Contributor

pierreglaser commented May 14, 2020

Hi @jjerphan, why was this closed? We should probably merge this before releasing joblib 0.15 / 1.0

@jjerphan
Copy link
Contributor Author

Hi @pierreglaser ; I just made some housekeeping on my stalled PRs, I reopen it now.

@jjerphan jjerphan reopened this May 14, 2020
@pierreglaser
Copy link
Contributor

@jjerphan If you find some time, feel free to finish this up :) we should release joblib early next week.

`check_pickle` should have been removed in 0.13.0 as indicated in the
`DeprecationWarning`.

This commit removes this argument, behavior, and the associated test.
@jjerphan jjerphan force-pushed the remove-check_pickle branch from 7c9a246 to 4da85fd Compare May 17, 2020 15:33
@codecov
Copy link

codecov bot commented May 17, 2020

Codecov Report

Merging #903 (264633b) into master (53a8173) will increase coverage by 0.62%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
+ Coverage   93.93%   94.56%   +0.62%     
==========================================
  Files          47       47              
  Lines        6956     6933      -23     
==========================================
+ Hits         6534     6556      +22     
+ Misses        422      377      -45     
Impacted Files Coverage Δ
joblib/test/test_parallel.py 97.09% <ø> (+0.17%) ⬆️
joblib/parallel.py 96.41% <100.00%> (-0.04%) ⬇️
joblib/test/test_memory.py 98.56% <0.00%> (+0.14%) ⬆️
joblib/pool.py 87.80% <0.00%> (+0.81%) ⬆️
joblib/func_inspect.py 92.26% <0.00%> (+1.19%) ⬆️
joblib/logger.py 86.84% <0.00%> (+1.31%) ⬆️
joblib/_parallel_backends.py 96.48% <0.00%> (+1.56%) ⬆️
joblib/disk.py 92.06% <0.00%> (+1.58%) ⬆️
joblib/test/test_memmapping.py 99.23% <0.00%> (+1.90%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53a8173...264633b. Read the comment docs.

@jjerphan
Copy link
Contributor Author

Hi @pierreglaser. I haven't managed to find a reason for the "failure" of the codecov pipeline.

@pierreglaser
Copy link
Contributor

Don't worry about the codecov complaints. This issue is affecting a lot of PRs, and I think joblib should use a coverage threshold like scikit-learn does.

@pierreglaser
Copy link
Contributor

pierreglaser commented May 18, 2020

@ogrisel if you want to take a look :) that would be nice to have this merged for 0.16.

@jjerphan
Copy link
Contributor Author

OK, thank you @pierreglaser for this piece of feedback.

@jjerphan
Copy link
Contributor Author

Small up.

Copy link
Contributor

@tomMoral tomMoral left a comment

Choose a reason for hiding this comment

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

This seems a bit overdue! thx @jjerphan.

Could you simply add an entry in the change log saying that you removed this deprecated argument?

@jjerphan
Copy link
Contributor Author

@tomMoral: thanks for the follow up! 🙂

CI checks on Azure are still failing but it is unrelated to those changes.

@ogrisel
Copy link
Contributor

ogrisel commented Nov 16, 2020

CI checks on Azure are still failing but it is unrelated to those changes.

It looks all green to me.

@ogrisel ogrisel merged commit 8c2cbd9 into joblib:master Nov 16, 2020
@ogrisel
Copy link
Contributor

ogrisel commented Nov 16, 2020

Thanks!

@tomMoral
Copy link
Contributor

Thanks!

@dmalzl
Copy link

dmalzl commented Feb 2, 2021

Hi there,

I am currently trying to run pomgranate to fit a hidden markov model to some data. This library uses joblib to parallelize the model training. However, when I try to train it I get the following error:

Empty                                     Traceback (most recent call last)
~/.conda/envs/mypython/lib/python3.6/site-packages/joblib/parallel.py in dispatch_one_batch(self, iterator)
    819             try:
--> 820                 tasks = self._ready_batches.get(block=False)
    821             except queue.Empty:

~/.conda/envs/mypython/lib/python3.6/queue.py in get(self, block, timeout)
    160                 if not self._qsize():
--> 161                     raise Empty
    162             elif timeout is None:

Empty:

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-7-dcc6125ae320> in <module>
----> 1 df = hmm(df, 3)

~/.conda/envs/mypython/lib/python3.6/site-packages/hmm_bigwigs/bigwig_hmm.py in hmm(df, num_states)
     40     vals = df["value"].values
     41     model = HiddenMarkovModel.from_samples(
---> 42         NormalDistribution, X=[vals], n_components=num_states
     43     )
     44     states = model.predict(vals)

~/.conda/envs/mypython/lib/python3.6/site-packages/pomegranate/hmm.pyx in pomegranate.hmm.HiddenMarkovModel.from_samples()

~/.conda/envs/mypython/lib/python3.6/site-packages/pomegranate/kmeans.pyx in pomegranate.kmeans.Kmeans.fit()

~/.conda/envs/mypython/lib/python3.6/site-packages/pomegranate/kmeans.pyx in genexpr()

~/.conda/envs/mypython/lib/python3.6/site-packages/joblib/parallel.py in __call__(self, iterable)
   1039             # remaining jobs.
   1040             self._iterating = False
-> 1041             if self.dispatch_one_batch(iterator):
   1042                 self._iterating = self._original_iterator is not None
   1043

~/.conda/envs/mypython/lib/python3.6/site-packages/joblib/parallel.py in dispatch_one_batch(self, iterator)
    829                 big_batch_size = batch_size * n_jobs
    830
--> 831                 islice = list(itertools.islice(iterator, big_batch_size))
    832                 if len(islice) == 0:
    833                     return False

~/.conda/envs/mypython/lib/python3.6/site-packages/pomegranate/kmeans.pyx in genexpr()

TypeError: delayed() got an unexpected keyword argument 'check_pickle'

I checked the input file and every variable I feed the model and all is fine there. I tried to find out where the 'check_pickle' kwarg came from but I was not able to find the culprit. Do you know what's going on?

Thanks

@jjerphan
Copy link
Contributor Author

jjerphan commented Feb 2, 2021

@dmalzl: the check_pickle has been removed from delayed in joblib. This has been fixed accordingly in pomegranate by jmschrei/pomegranate@42d14be. Updating pomgranate to v0.14.0 should fix the problem:

pip install - U pomegranate

@dmalzl
Copy link

dmalzl commented Feb 2, 2021

Ah okay. Since I just installed it today, I thought I already had the newest version. Seems the conda recipe is outdated then. That's what you get from blind trust.

Anyway, works now after updating.
Thanks for the help

@jjerphan
Copy link
Contributor Author

jjerphan commented Feb 2, 2021

You're welcome.

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.

5 participants