Address some mypy type hinting issues

[AitElBadaoui]

Summary of Changes

This merge request addresses some of the type hinting issues identified by MyPy.
Ref: #782.

Requirements

• My PR is focused on a single feature addition or bugfix.
• My PR has relevant, comprehensive unit tests.
• My PR is on a custom branch (i.e. is not named main).

Notes

• Before contributing for the first time, read the "Guidelines" section.
• If you are an external contributor, you will see a comment from @buildbot-princeton. This is solely for the maintainers.
• When your code is ready for review, ping one of the active maintainers.

[buildbot-princeton]

Can one of the admins verify this patch?

[codecov]

Codecov Report

Attention: Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.06%. Comparing base (afde9a8) to head (0240058).

Files	Patch %	Lines
src/quacc/recipes/gulp/_base.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2314      +/-   ##
==========================================
- Coverage   99.08%   99.06%   -0.03%     
==========================================
  Files          84       84              
  Lines        3515     3519       +4     
==========================================
+ Hits         3483     3486       +3     
- Misses         32       33       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[AitElBadaoui]

Hello @Andrew-S-Rosen
The missing line in the coverage is because none of the jobs call the run_and_summarize function with a None value in option_defaults. I'm not sure if it's worth it to mock a job just to cover this one line of code. Please let me know what you think.

To resolve the remaining MyPy issues, I need to understand the objective of this MR: #1903. The changes introduced have led to multiple MyPy errors due to assigning tuples as default values to variables of different data types. Is _DEFAULT_SETTING meant to always be empty ?

Thank you !

[Andrew-S-Rosen]

Hi @AitElBadaoui! Thank you so much for putting together this PR. I really appreciate it!! These all look like great changes and, in fact, may even reduce some bugs.

As for the test coverage, don't worry about it for this PR. No need to worry about test coverage for type hinting changes here.

As for PR #1903, I can see how that might introduce some problems in hindsight. The reasoning behind the PR was that I thought _DEFAULT_SETTING (rather than None) would make it clearer to the end-user that a default setting was being used (quacc has a set of default settings it imports at runtime). However, I see the problem with the type hinting. I chose to use () as a placeholder because () was simple and immutable. I also thought this approach would be nice because then None could actually mean something distinct from using the defaults. But the problem is that something like qchem_cmd: str = _DEFAULT_SETTING is clearly going to be flagged as a problem by mypy since _DEFAULT_SETTING is a tuple (even though the actual setting being used at runtime is indeed a str).

I guess since the type hint is now "wrong", would you recommend that I switch back to None in those cases? I can do that in a separate PR if you think that is the best approach.

[Andrew-S-Rosen]

As for a review, here are my (minor) comments. Otherwise, this is in great shape for merging!

[AitElBadaoui]

Yes, I agree that we should switch back to None. After that, I can create a new PR to address other mypy issues.
Thank you for the review!

[Andrew-S-Rosen]

@AitElBadaoui: wonderful! I will take care of that and merge this PR when I'm back at my computer in an hour or two. 👍

[Andrew-S-Rosen]

Merging this one. Thanks!