Skip to content
Please note that GitHub no longer supports your web browser.

We recommend upgrading to the latest Google Chrome or Firefox.

Learn more
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 some PySpark udfs #412

Draft
wants to merge 8 commits into
base: master
from
Draft

Add some PySpark udfs #412

wants to merge 8 commits into from

Conversation

@ruebot
Copy link
Member

ruebot commented Jan 18, 2020

GitHub issue(s): #409

What does this Pull Request do?

  • Add remove_http_header, remove_prefix_www
  • Rename extract_domain_func to extract_domain
  • Formatting updates
  • Addresses #409

How should this be tested?

https://gist.github.com/ruebot/e50892b0b2b4a6abad8ddc7933cf79b2

Additional Notes:

  • We need to sort out how we'll bundle something like everything that requirements.txt would help with in the zip file; something like the Uberjar. Right now, we really don't have anything, but I imagine we'd want to pull in external libraries like Beautiful Soup, or tld-extractor.

  • I'll leave this as a draft, and push to it as I'm working on it. Others are welcome to push to it as well, since GitHub is now setup to provide credit to all those accounts who contributed to a PR when it is squashed down.

Interested parties

@SinghGursimran if you're sick of Scala, let me know, and I can give you access.

@lintool this approach fine? Naming convention fine?

@ianmilligan1 let me know if the notebook testing makes sense. Figured that'd be easy to test this stuff.

- Add remove_http_header, remove_prefix_www
- Rename extract_domain_func to extract_domain
- Formatting updates
- Addresses #409
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jan 18, 2020

Codecov Report

Merging #412 into master will increase coverage by 0.14%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #412      +/-   ##
=========================================
+ Coverage   77.56%   77.7%   +0.14%     
=========================================
  Files          40      40              
  Lines        1542    1552      +10     
  Branches      292     292              
=========================================
+ Hits         1196    1206      +10     
  Misses        218     218              
  Partials      128     128
@SinghGursimran

This comment has been minimized.

Copy link
Collaborator

SinghGursimran commented Jan 18, 2020

@ruebot, yes, sure, pls give the access.

@ianmilligan1

This comment has been minimized.

Copy link
Member

ianmilligan1 commented Jan 18, 2020

@ianmilligan1 let me know if the notebook testing makes sense. Figured that'd be easy to test this stuff.

Makes sense to me! Thanks @ruebot.

ruebot and others added 3 commits Jan 18, 2020
g285sing
g285sing
@ruebot

This comment has been minimized.

Copy link
Member

ruebot commented on src/main/python/aut/udfs.py in 5830ab9 Jan 19, 2020

I have a version of this with BeautifulSoup on my local branch that I haven't pushed up yet. That might work better, we can tests and see which method works better later. In the interim, I'm close to a packaging solution with dependencies on my branch that I'll hopefully be able to push up soon. Then we can pull in external libraries and make life a whole lot easier 😃

This comment has been minimized.

Copy link
Collaborator

SinghGursimran replied Jan 19, 2020

Great!!

ruebot added 3 commits Jan 21, 2020
Add remove_html udf
Rename remove_http_header to remove_http_headers
…ue-409
- Get detect_language setup
- ComputeSHA1 and MD5 need some work?
@ruebot

This comment has been minimized.

Copy link
Member Author

ruebot commented Jan 21, 2020

Updated the testing notebook. Hopefully it renders. It might be getting too big with content in it 🤷‍♂

https://gist.github.com/ruebot/e50892b0b2b4a6abad8ddc7933cf79b2

return content.replace("[\\r\\n]+", " ")


remove_html_no_external_lib = udf(remove_html_no_external_lib, StringType())

This comment has been minimized.

Copy link
@ruebot

ruebot Jan 21, 2020

Author Member

I think we end up with HTML still in this one after looking at the output. You cool if we remove it and go the Beautifulsoup route?

from textblob import TextBlob


def compute_MD5(bytes):

This comment has been minimized.

Copy link
@ruebot

ruebot Jan 21, 2020

Author Member

I'm getting a TypeError: Unicode-objects must be encoded before hashing on this and compute_MD5.

def extract_domain_func(url):

def detect_language(input):
text = TextBlob(input)

This comment has been minimized.

Copy link
@ruebot

ruebot Jan 21, 2020

Author Member

This doesn't look like it's going to scale: "Language translation and detection powered by Google Translate"

I ran it twice in testing on the 10 Geocities files I use locally, and I got hit with a: urllib.error.HTTPError: HTTP Error 429: Too Many Requests.

Which all begs a bigger question on porting a lot of these over to Python; do we need to? We provide an MD5, SHA1, and Language column now. Is there a use case for having them in Python? I can't think of a reason to run them on a column if we already provide them. @ianmilligan1 @lintool what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.