Add linting tests with Aqua, JET and ExplicitImports, fix bugs

[adrhill]

This PR adds linting tests with Aqua.jl, JET.jl and ExplicitImports.jl and fixes the bugs that were caught by them:

• answer_box called a non-existing function answer
• fix bug caused by a typo: qwrite instead of write
• make previously unused GermanGermanyFormal available
• remove unused dependencies LaTeXStrings and Random
• fix missing compat entries for Markdown.jl and Downloads.jl (closes CompatHelper: add new compat entry for Downloads at version 1, (keep existing compat) #43)

I've also taken the liberty to move all exports to src/PlutoTeachingTools.jl.

[eford]

Thanks for helping with the cleanup.

I'd like to suggest keeping
using LaTeXStrings
so that plotting just works without users having to include that. Or at least consider it a breaking change for 1.0.

Also, we should probably keep Random for now for the same reason. (I know we were using rand() for choosing since response options. But I forget why we added Random.)

[adrhill]

I'd like to suggest keeping using LaTeXStrings so that plotting just works without users having to include that. [...]
Also, we should probably keep Random for now for the same reason.

As far as I can tell, LaTeXStrings and Random previously weren't reexported, so removing them shouldn't have any effect on outside code (?)

I know we were using rand() for choosing since response options. But I forget why we added Random.

rand() is used in confetti, but part of Julia Base, not Random.jl.
Removing it doesn't break any tests and the example notebook seems to render correctly without errors.

[adrhill]

Oh, and while we're at it @eford, is there any reason the internationalizations/translations are defined in submodules?

[eford]

I'd like to suggest keeping using LaTeXStrings so that plotting just works without users having to include that. [...]
Also, we should probably keep Random for now for the same reason.

As far as I can tell, LaTeXStrings and Random previously weren't reexported, so removing them shouldn't have any effect on outside code (?)

Ok. I find the syntax for accessing packages used inside other packages confusing, so I may have tried but failed to save people the need to findout about/explicitly include LaTeXStrings.
In retrospect, I think the bigger barrier is finding out that LaTeXStrings exists rather than including using LaTeXStrings. So maybe we can better meet this need by adding to the README a list of optional packages that are commonly useful.

I know we were using rand() for choosing since response options. But I forget why we added Random.

rand() is used in confetti, but part of Julia Base, not Random.jl. Removing it doesn't break any tests and the example notebook seems to render correctly without errors.

Ok. I probably I saw some people explicitly using Random and thought maybe we were supposed to do that to future proof code.

[eford]

Oh, and while we're at it @eford, is there any reason the internationalizations/translations are defined in submodules?

I think my original idea was that would make it easy for people to just specify the one submodule they wanted for their notebook to use.
Later I realized that some people wanted to write notebooks that could dynamically switch between languages. So I added support for that.

Another factor was that I figured different people would likely contribute different languages, so keeping them in separate files and namespaces would make it easier to add new languages and avoid accidentally breaking code from another language.

[eford]

@adrhill I saw in another PR where you were waiting for approval. Do you have someone in particular in mind?
This started as a solo project, so we haven't been following best practices of having someone different approve merges or new versions.
I appreciate the intent of changes you listed, but wasn't planning to review individual lines of code.

[eford]

What does v <0.0.1 mean? Why not just set this to "1"? Same for Markdown

[adrhill]

Markdown.jl and Downloads.jl are standard libraries and used to be coupled to their Julia releases. Now they are stand-alone packages and have version numbers. For backward compatibility, these new compat entries have to include <0.0.1. The 1 is the latest stable major release of Downloads.jl (currently 1.6.0).

You can find out more here: https://discourse.julialang.org/t/psa-compat-requirements-in-the-general-registry-are-changing/104958

[eford]

I don't know much about these, so I'm just trusting that they're workingl

[adrhill]

I should have gone into more detail in the PR description – sorry about that!

JuliaFormatter.jl

JuliaFormatter.jl is the most commonly used Julia code formatter. I've added a .JuliaFormatter.toml that specifies the Blue code style in #47. Code can be formatted by calling using JuliaFormatter; format(".") or by using the formatting command in VSCode. JuliaFormatter can also be used as a formatting check in the package tests. This way, we ensure that every PR commits formatted code.

Aqua.jl

Aqua.jl runs several automated tests:

• There are no method ambiguities.
• There are no undefined exports.
• There are no unbound type parameters.
• There are no stale dependencies listed in Project.toml.
• Check that test target of the root project Project.toml and test project (test/Project.toml) are consistent.
• Check that all external packages listed in deps have corresponding compat entries.
• There are no "obvious" type piracies.

JET.jl

JET.jl automatically tests code for type instabilities (and in my experience also catches a bunch of other bugs).

ExplicitImports.jl

ExplicitImports.jl is a rather new project and tests proper import and export "hygiene". With the introduction of the new public keyword, Julia developers will hopefully avoid using non-public internals.

[greimel]

@adrhill If you set up these style checks, you should also report what exactly doesn't satisfy the rules. Otherwise you create a barrier to contributions. I've not set up JuliaFormatter locally and I don't intend to do so.

[greimel]

my comment is addressed here: #53

[adrhill]

Yes, this is a valid concern. We might also want to link to the ColPrac guide to help newcomers.

I've not set up JuliaFormatter locally and I don't intend to do so.

You probably do have JuliaFormatter available within VSCode:

If you don't use VSCode, it is part of the PlutoTeachingTools test environment, which can be activated via ]activate test.

The advantages and disadvantages of code formatters have been discussed to death. gofmt has been a big success in the Go community. If all code you read and write in a language is formatted in a standardized way, it arguably lowers the barrier to contributions.

[greimel]

I am all for formatted code. What I am against is marking PRs with a red x with no instructions how to fix it.

This should be fixed if #53 works as intended.

[eford]

Thanks a bunch! A few minor comments included.

[adrhill]

I saw in another PR where you were waiting for approval. Do you have someone in particular in mind? [...]
I appreciate the intent of changes you listed, but wasn't planning to review individual lines of code.

I use PlutoTeachingTools in my Julia programming for Machine Learning course and have some free time in my vacation to spend on open source projects, so I thought it would be well invested here.
I feel like it would be bad practice (and rude?) to merge my own changes when they are non-trivial. Especially since this started as your solo project. :)

Later I realized that some people wanted to write notebooks that could dynamically switch between languages. So I added support for that.

Yeah, as far as I can tell, PlutoTeachingTools automatically selects the language based on the system language of a computer, which is very nice!

So maybe we can better meet this need by adding to the README a list of optional packages that are commonly useful.

This is a great idea!
EDIT: I've added it to the LaTeX section of the example notebook.

[eford]

I saw in another PR where you were waiting for approval. Do you have someone in particular in mind? [...]
I appreciate the intent of changes you listed, but wasn't planning to review individual lines of code.

I use PlutoTeachingTools in my Julia programming for Machine Learning course and have some free time in my vacation to spend on open source projects, so I thought it would be well invested here.

I'm very glad to hear it's been useful for you. Thanks for the link. Maybe we should start a list of courses using PlutoTeachingTools in the Readme.

I feel like it would be bad practice (and rude?) to merge my own changes when they are non-trivial. Especially since this started as your solo project. :)

Thanks for the consideration. You're right... it's definitely not the best practice if you have a sizable software development team to support it. If there's just a few of us maintaining it as a hobby, then I think might use "good enough" practices.

Later I realized that some people wanted to write notebooks that could dynamically switch between languages. So I added support for that.

Yeah, as far as I can tell, PlutoTeachingTools automatically selects the language based on the system language of a computer, which is very nice!

I'm glad someone appreciates it.

So maybe we can better meet this need by adding to the README a list of optional packages that are commonly useful.

This is a great idea! EDIT: I've added it to the LaTeX section of the example notebook.

Excellent!

[eford]

Thanks!