MNT integrate pickle.save_global logic in cloudpickle natively

[pierreglaser]

This PR solves #261. It is also preparation work for #253 .

Preparation work for #253

Previously, checking if a class was pickleable was done by:

try:
    Pickler.save_global(self, obj, name=name)  # call pickle._Pickler.save_global
except Exception:
    # obj is not pickleable
    ... 

We cannot do that in #272 as save_global is not an accessible method of the C accelerated Pickler. We could do

try:
    pickle.dumps(obj)
except Exception:
   ...

But this is quite costly as it recreates a Pickler from scratch.

Instead, we can re-write the logic of _Pickler.save_global into cloudpickle. It was partially already there in cloudpickle.Cloudpickler.save_function. I extracted this logic in a new _is_global helper. Also, I used it both for classes and functions, because the difference is not made in pickle.Pickler.save_global: both of those types are treated as global.

Solving #261

Now that the pickling logic of save_global is natively implemented in cloudpickle, we can handle exceptions at a finer grain. Especially, we can ignore Exceptions coming from unrelated modules when resolving an object's module attribute path, while not ignoring Errors coming from the object's actual module.

More cleanups

I also removed some unreachable code in save_function.
These spreadsheets split down all possible use-cases into 4 subspaces. The number are, for each subspace, the line at which save_function returns. In any subspace, save_function exits at a line preceding these lines:

cloudpickle/cloudpickle/cloudpickle.py

Lines 440 to 449 in 6176193

	if obj.__dict__:
	# essentially save_reduce, but workaround needed to avoid recursion
	self.save(_restore_attr)
	write(pickle.MARK + pickle.GLOBAL + modname + '\n' + name + '\n')
	self.memoize(obj)
	self.save(obj.__dict__)
	write(pickle.TUPLE + pickle.REDUCE)
	else:
	write(pickle.GLOBAL + modname + '\n' + name + '\n')
	self.memoize(obj)

So it is safe to remove them.

[codecov]

Codecov Report

Merging #273 into master will increase coverage by 0.83%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   89.19%   90.03%   +0.83%     
==========================================
  Files           1        1              
  Lines         611      602       -9     
  Branches      127      127              
==========================================
- Hits          545      542       -3     
+ Misses         43       39       -4     
+ Partials       23       21       -2

Impacted Files	Coverage Δ
cloudpickle/cloudpickle.py	90.03% <89.47%> (+0.83%)	⬆️

---

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 6176193...d1b96c5. Read the comment docs.

[codecov]

Codecov Report

Merging #273 into master will increase coverage by 2.38%.
The diff coverage is 98%.

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   89.19%   91.58%   +2.38%     
==========================================
  Files           1        1              
  Lines         611      606       -5     
  Branches      127      129       +2     
==========================================
+ Hits          545      555      +10     
+ Misses         43       31      -12     
+ Partials       23       20       -3

Impacted Files	Coverage Δ
cloudpickle/cloudpickle.py	91.58% <98%> (+2.38%)	⬆️

---

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 6176193...ff61f6c. Read the comment docs.

[ogrisel]

This looks good to me. The joblib downstream failure might be unrelated to cloudpickle but we have to investigate whether or not this is the case.

[ogrisel]

LGTM, just a simple request:

[ogrisel]

Thanks @pierreglaser, the code is much easier to follow this way.