Aut: PEP8 Naming - UDFs, App method names, DataFrame names, and filters.

Created on 27 May 2020  ·  10Comments  ·  Source: archivesunleashed/aut

Let's evaluate how we've named everything; UDFs, App method names, DataFrame names, and filters.

Are we all good with how we've named everything?

Do we need to change anything before a 1.0.0 release?

See also #467

discussion

Most helpful comment

My thoughts: In Python, I would default to PEP8 unless there's a convincing reason otherwise: https://www.python.org/dev/peps/pep-0008/

Apps are most like Classes, so I would go CamelCase. Note, it says:

Note: When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.

UDFs are like methods, so should follow standard snake_case.

All 10 comments

My thoughts: In Python, I would default to PEP8 unless there's a convincing reason otherwise: https://www.python.org/dev/peps/pep-0008/

Apps are most like Classes, so I would go CamelCase. Note, it says:

Note: When using acronyms in CapWords, capitalize all the letters of the acronym. Thus HTTPServerError is better than HttpServerError.

UDFs are like methods, so should follow standard snake_case.

@lintool anything on the Scala side? If not, I'll get some PRs ready for aut, documentation, and notebooks.

  • ExtractPopularImages
  • WriteGexf
  • WriteGraphml

Feel free to vote down if you think this ship has already sailed... Currently, we have removeHTML and remove_html for DF UDFs in Scala and Python, respective. This is good.

However, for RDDs, we still have RemoveHTMLRDD. I suppose we can rename those to just RemoveHTML. The casing differences make it clear that small camel is DF and regular camel is RDD?

And also: https://github.com/archivesunleashed/aut-docs/blob/master/current/dataframe-schemas.md

Thoughts about Python/Scala naming conventions? Right now we use small camel case. In Python DF, this goes against the best practice of downcased snake case.

There are three relevant cases:

  • .presentationProgramFiles()
  • .wordProcessorFiles()
  • .textFiles()

If we change to:

  • .presentations()
  • .documents()
  • .textfiles()

We would completely resolve any of these issues. .documents() might be slightly confusing; .textfiles() seems fine to me as all lower cased since we have .imagegraph().


And with this, I think I've said my peace on naming.

Good call on the Scala side. It'll be nice to remove those RDD suffixes. I'll get the Scala side sorted out here in a bit.

For the three additional cases, these two -- presentationProgramFiles and wordProcessorFiles -- their collective names come directly from Wikipedia, and I'm fairly certain I outlined all this the last time it came up in an issue. As you say, documents is too confusing. I'd rather be fairly explicit with this, rather than vague. On the Python side, word_processor and presentation_program, so I could see changing the Scala side to wordProcessor and presentationProgram. I think that's a good compromise.

As for textFiles vs textfiles, I'm happy to make that change, and even happier to just yank it completely since they're so hard to nail now. We get way too many false positives on textFiles() extraction jobs. But, then again, it could be helpful for research questions :man_shrugging:

Good call on the Scala side. It'll be nice to remove those RDD suffixes. I'll get the Scala side sorted out here in a bit.

👍

For the three additional cases, these two -- presentationProgramFiles and wordProcessorFiles -- their collective names come directly from Wikipedia, and I'm fairly certain I outlined all this the last time it came up in an issue. As you say, documents is too confusing. I'd rather be fairly explicit with this, rather than vague. On the Python side, word_processor and presentation_program, so I could see changing the Scala side to wordProcessor and presentationProgram. I think that's a good compromise.

I think I'm fine with presentationProgramFiles and wordProcessorFiles on the Scala side and presentation_program_files and word_processor_files on the Python side. I think this is a bit clearer - we mean _files_ from those application, not those applications themselves.

As for textFiles vs textfiles, I'm happy to make that change, and even happier to just yank it completely since they're so hard to nail now. We get way too many false positives on textFiles() extraction jobs. But, then again, it could be helpful for research questions 🤷‍♂️

Let's yank.

@lintool #479 _should_ address all of this.

Should be closed with 1ac97ef4981ebd31dd14e4ed08101eb22da15bbd (#479).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ruebot picture ruebot  ·  7Comments

lintool picture lintool  ·  6Comments

ruebot picture ruebot  ·  5Comments

SamFritz picture SamFritz  ·  4Comments

ruebot picture ruebot  ·  4Comments