[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/18734 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18734#discussion_r133609212
--- Diff: python/pyspark/cloudpickle.py ---
@@ -241,11 +338,32 @@ def save_function(self, obj, name=None):
if getattr(themodule, name, None) is obj:
return self.save_global(obj, name)
+# a builtin_function_or_method which comes in as an attribute of
some
+# object (e.g., object.__new__, itertools.chain.from_iterable)
will end
+# up with modname "__main__" and so end up here. But these
functions
+# have no __code__ attribute in CPython, so the handling for
+# user-defined functions below will fail.
+# So we pickle them here using save_reduce; have to do it
differently
+# for different python versions.
+if not hasattr(obj, '__code__'):
+if PY3:
+if sys.version_info < (3, 4):
+raise pickle.PicklingError("Can't pickle %r" % obj)
+else:
+rv = obj.__reduce_ex__(self.proto)
+else:
+if hasattr(obj, '__self__'):
+rv = (getattr, (obj.__self__, name))
+else:
+raise pickle.PicklingError("Can't pickle %r" % obj)
+return Pickler.save_reduce(self, obj=obj, *rv)
+
# if func is lambda, def'ed at prompt, is in main, or is nested,
then
# we'll pickle the actual function object rather than simply
saving a
# reference (as is done in default pickler), via
save_function_tuple.
-if islambda(obj) or obj.__code__.co_filename == '' or
themodule is None:
-#print("save global", islambda(obj), obj.__code__.co_filename,
modname, themodule)
+if (islambda(obj)
--- End diff --
Just as a side note, it looks this PR includes
https://github.com/cloudpipe/cloudpickle/pull/51 too (SPARK-21753).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/18734#discussion_r132334455
--- Diff: python/pyspark/cloudpickle.py ---
@@ -397,42 +625,7 @@ def save_global(self, obj, name=None,
pack=struct.pack):
typ = type(obj)
if typ is not obj and isinstance(obj, (type, types.ClassType)):
-d = dict(obj.__dict__) # copy dict proxy to a dict
-if not isinstance(d.get('__dict__', None), property):
-# don't extract dict that are properties
-d.pop('__dict__', None)
-d.pop('__weakref__', None)
-
-# hack as __new__ is stored differently in the __dict__
-new_override = d.get('__new__', None)
-if new_override:
-d['__new__'] = obj.__new__
-
-# workaround for namedtuple (hijacked by PySpark)
-if getattr(obj, '_is_namedtuple_', False):
-self.save_reduce(_load_namedtuple, (obj.__name__,
obj._fields))
-return
-
-self.save(_load_class)
-self.save_reduce(typ, (obj.__name__, obj.__bases__,
{"__doc__": obj.__doc__}), obj=obj)
-d.pop('__doc__', None)
-# handle property and staticmethod
-dd = {}
-for k, v in d.items():
--- End diff --
Gentle re-ping to @davies - do you have an opinion on this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user holdenk commented on a diff in the pull request: https://github.com/apache/spark/pull/18734#discussion_r130774005 --- Diff: python/pyspark/cloudpickle.py --- @@ -220,12 +322,7 @@ def save_function(self, obj, name=None): if name is None: name = obj.__name__ -try: -# whichmodule() could fail, see -# https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling -modname = pickle.whichmodule(obj, name) -except Exception: -modname = None +modname = pickle.whichmodule(obj, name) --- End diff -- It's a good question, the underlying issue was marked resolved in 2014 and from looking at the commit it seems like it should actually be resolved. That being said its true some people might be on a system installed with an old verison of six so perhaps we should keep this workaround. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/18734#discussion_r130773575
--- Diff: python/pyspark/cloudpickle.py ---
@@ -397,42 +625,7 @@ def save_global(self, obj, name=None,
pack=struct.pack):
typ = type(obj)
if typ is not obj and isinstance(obj, (type, types.ClassType)):
-d = dict(obj.__dict__) # copy dict proxy to a dict
-if not isinstance(d.get('__dict__', None), property):
-# don't extract dict that are properties
-d.pop('__dict__', None)
-d.pop('__weakref__', None)
-
-# hack as __new__ is stored differently in the __dict__
-new_override = d.get('__new__', None)
-if new_override:
-d['__new__'] = obj.__new__
-
-# workaround for namedtuple (hijacked by PySpark)
-if getattr(obj, '_is_namedtuple_', False):
-self.save_reduce(_load_namedtuple, (obj.__name__,
obj._fields))
-return
-
-self.save(_load_class)
-self.save_reduce(typ, (obj.__name__, obj.__bases__,
{"__doc__": obj.__doc__}), obj=obj)
-d.pop('__doc__', None)
-# handle property and staticmethod
-dd = {}
-for k, v in d.items():
--- End diff --
Lets double check this part with @davies.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/18734#discussion_r130763298 --- Diff: python/pyspark/cloudpickle.py --- @@ -220,12 +322,7 @@ def save_function(self, obj, name=None): if name is None: name = obj.__name__ -try: -# whichmodule() could fail, see -# https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling -modname = pickle.whichmodule(obj, name) -except Exception: -modname = None +modname = pickle.whichmodule(obj, name) --- End diff -- @holdenk, would it be safe to remove? It looks a custom fix we did to deal with https://issues.apache.org/jira/browse/SPARK-16077 and sounds we should keep this as https://bitbucket.org/gutworth/six/issues/63/importing-six-breaks-pickling describes Python 2.7. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18734#discussion_r130765110
--- Diff: python/pyspark/cloudpickle.py ---
@@ -623,72 +816,82 @@ def save_file(self, obj):
return self.save_reduce(getattr, (sys,'stderr'), obj=obj)
if obj is sys.stdin:
raise pickle.PicklingError("Cannot pickle standard input")
-if hasattr(obj, 'isatty') and obj.isatty():
+if obj.closed:
+raise pickle.PicklingError("Cannot pickle closed files")
+if hasattr(obj, 'isatty') and obj.isatty():
raise pickle.PicklingError("Cannot pickle files that map to
tty objects")
-if 'r' not in obj.mode:
-raise pickle.PicklingError("Cannot pickle files that are not
opened for reading")
+if 'r' not in obj.mode and '+' not in obj.mode:
+raise pickle.PicklingError("Cannot pickle files that are not
opened for reading: %s" % obj.mode)
+
name = obj.name
-try:
-fsize = os.stat(name).st_size
-except OSError:
-raise pickle.PicklingError("Cannot pickle file %s as it cannot
be stat" % name)
-if obj.closed:
-#create an empty closed string io
-retval = pystringIO.StringIO("")
-retval.close()
-elif not fsize: #empty file
-retval = pystringIO.StringIO("")
-try:
-tmpfile = file(name)
-tst = tmpfile.read(1)
-except IOError:
-raise pickle.PicklingError("Cannot pickle file %s as it
cannot be read" % name)
-tmpfile.close()
-if tst != '':
-raise pickle.PicklingError("Cannot pickle file %s as it
does not appear to map to a physical, real file" % name)
-else:
-try:
-tmpfile = file(name)
-contents = tmpfile.read()
-tmpfile.close()
-except IOError:
-raise pickle.PicklingError("Cannot pickle file %s as it
cannot be read" % name)
-retval = pystringIO.StringIO(contents)
+retval = pystringIO.StringIO()
+
+try:
+# Read the whole file
curloc = obj.tell()
-retval.seek(curloc)
+obj.seek(0)
+contents = obj.read()
+obj.seek(curloc)
+except IOError:
+raise pickle.PicklingError("Cannot pickle file %s as it cannot
be read" % name)
+retval.write(contents)
+retval.seek(curloc)
retval.name = name
self.save(retval)
self.memoize(obj)
+def save_ellipsis(self, obj):
+self.save_reduce(_gen_ellipsis, ())
+
+def save_not_implemented(self, obj):
+self.save_reduce(_gen_not_implemented, ())
+
if PY3:
dispatch[io.TextIOWrapper] = save_file
else:
dispatch[file] = save_file
-"""Special functions for Add-on libraries"""
+dispatch[type(Ellipsis)] = save_ellipsis
+dispatch[type(NotImplemented)] = save_not_implemented
-def inject_numpy(self):
--- End diff --
I assume removing this is fine per
https://github.com/cloudpipe/cloudpickle/commit/1e91fa7c0f9b1e77604d83b3ba9aecde8603ece1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18734: [SPARK-21070][PYSPARK] Attempt to update cloudpic...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18734#discussion_r130766049
--- Diff: python/pyspark/cloudpickle.py ---
@@ -397,42 +625,7 @@ def save_global(self, obj, name=None,
pack=struct.pack):
typ = type(obj)
if typ is not obj and isinstance(obj, (type, types.ClassType)):
-d = dict(obj.__dict__) # copy dict proxy to a dict
-if not isinstance(d.get('__dict__', None), property):
-# don't extract dict that are properties
-d.pop('__dict__', None)
-d.pop('__weakref__', None)
-
-# hack as __new__ is stored differently in the __dict__
-new_override = d.get('__new__', None)
-if new_override:
-d['__new__'] = obj.__new__
-
-# workaround for namedtuple (hijacked by PySpark)
-if getattr(obj, '_is_namedtuple_', False):
-self.save_reduce(_load_namedtuple, (obj.__name__,
obj._fields))
-return
-
-self.save(_load_class)
-self.save_reduce(typ, (obj.__name__, obj.__bases__,
{"__doc__": obj.__doc__}), obj=obj)
-d.pop('__doc__', None)
-# handle property and staticmethod
-dd = {}
-for k, v in d.items():
--- End diff --
I assume we are fine per the tests added in
https://github.com/apache/spark/commit/e044705b4402f86d0557ecd146f3565388c7eeb4.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
