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

Rename DF functions to be consistent with Python DF functions. #379

Merged
merged 1 commit into from Nov 17, 2019

Conversation

@ruebot
Copy link
Member

ruebot commented Nov 14, 2019

GitHub issue(s): #366

What does this Pull Request do?

Rename DF functions to be consistent with Python DF functions.

How should this be tested?

  • TravisCI (unit tests) should take care of it.

Additional Notes:

  1. @lintool the only part of the issue I haven't resolved here is your proposal to move WebArchive to RecordLoader.loadArchives on the Python side. I don't believe we can have RecordLoader.loadArchives as an actual class name. So, should we change things in the other direction or both directions? I've been naming a val warcs a lot in my example. What if we did something like that? Or webArchives or webarchives ? Once we settle on something, I'll the PR out of draft.

  2. There will be a TONNE of documentation updates to do. If I could get a hand there, that'd be super helpful. @SamFritz @ianmilligan1 that of any interest?

- Resolves #366
@ruebot ruebot requested a review from lintool Nov 14, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #379 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #379   +/-   ##
=======================================
  Coverage   76.16%   76.16%           
=======================================
  Files          40       40           
  Lines        1418     1418           
  Branches      268      268           
=======================================
  Hits         1080     1080           
  Misses        221      221           
  Partials      117      117
@ruebot ruebot added this to In review in DataFrames and PySpark Nov 14, 2019
@ruebot ruebot added this to In Progress in 1.0.0 Release of AUT Nov 14, 2019
@ianmilligan1

This comment has been minimized.

Copy link
Member

ianmilligan1 commented Nov 15, 2019

👀

Looks like a great PR in progress, and agreed, the documentation will be heavy-ish lifting. I can help out if you give me discrete chunks of work (between parental leave, grant writing, admin, and some unfortunate university admin work coming up, my time is a bit limited, but many hands makes for light work).

@lintool

This comment has been minimized.

Copy link
Member

lintool commented Nov 15, 2019

@rubot I'm not wedded to perfect alignment between Scala DF and Pyspark DF at this point... perhaps we should just wait for #371?

I would rather do the refactoring incrementally... i.e., let's get this merged, start fixing docs, etc.

@ruebot

This comment has been minimized.

Copy link
Member Author

ruebot commented Nov 15, 2019

@lintool cool 🤘 🤘

@ruebot ruebot marked this pull request as ready for review Nov 15, 2019
@ianmilligan1 ianmilligan1 merged commit 67ca17d into master Nov 17, 2019
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 76.16%)
Details
codecov/project 76.16% (+0%) compared to f9ce826
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
1.0.0 Release of AUT automation moved this from In Progress to Done Nov 17, 2019
@ianmilligan1 ianmilligan1 deleted the issue-366 branch Nov 17, 2019
@ianmilligan1

This comment has been minimized.

Copy link
Member

ianmilligan1 commented Nov 17, 2019

Great! @ruebot and I can maybe work together or the docs? When I have time I can start a branch or vice versa.

@ruebot

This comment has been minimized.

Copy link
Member Author

ruebot commented Nov 17, 2019

Cool. I'll update archivesunleashed/aut-docs-new#21 with the new names, then we can go from there? Or, if you want to start on another file, go for it. Shouldn't be too much besides that hopefully.

@ianmilligan1

This comment has been minimized.

Copy link
Member

ianmilligan1 commented Nov 17, 2019

Sure, sounds good on updating the PR and going fro, there! I might be able to tackle some of this tomorrow, kids schedule permitting.

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