Lint Loads

[ssolson]

This PR is part of an ongoing effort to enhance the MHKiT codebase's quality and maintainability, specifically targeting the loads module. Our main objectives were to ensure Pylint compliance and to enrich the code with type hints across all functions within this module. Here's a summary of the key changes and considerations:

Pylint Compliance

Revised the loads module to adhere to Pylint's coding standards. This involved a comprehensive review and subsequent adjustments to formatting, naming conventions, and structure to meet the stringent requirements set forth by Pylint.

A new Pylint test can be seen on this PR that enforces Pylint compliance on just the loads module.

Type Hints Addition

Type hints have been added to each function within the module, providing clearer documentation and facilitating better IDE support for type checking. This enhancement is expected to improve code readability and maintainability significantly.

Exceptions & Disables

While striving for Pylint compliance, we've encountered a few instances where adhering strictly to Pylint's recommendations would either compromise the codebase's integrity or cause excessive delays. In these cases, we've opted to disable specific Pylint checks, detailed as follows:

• Excessive Arguments: For functions flagged with R0913 (too many arguments) and R0914 (too many local variables), we've allowed exceptions to maintain the current functionality and interface.
• Special Case Variable n: In the LongTermExtreme function, the variable name n is retained with a disable directive to avoid unforeseen impacts on the code due to renaming.
• Incorrect Warning W0632: Pylint mistakenly flagged a function for returning fewer arguments than expected. We've disabled this check after thorough verification.
• Method Argument Conflict: The _Peaks class's cdf method required a disable for arguments-differ to resolve a conflict with the built-in rv_continuous method's signature.

[ssolson]

@akeeste this should be ready for a review. I pushed really hard to get this finished before being OOO next week. I hope I didn't miss anything but I believe I have all my bases covered for a review.

[akeeste]

sorry for the wait here, I finally got #285 ironed out and ready to go. I blocked time to review this PR first thing next week.

[ssolson]

thanks for this PR @ssolson. Overall I think these pylint changes look pretty good, though it's hard to review the loads.extreme since it was broken into multiple files. Is there any specific changes to review for those functions?

I think your to_numeric_array function is actually a good complement to the utils.data_utils functions that convert function input to xarray format. There is some overlap, but I also see use cases for both right now--especially in graphics functions that require a more simplified numeric format, or function like those in loads.extreme.extremes that were previously built to only take numpy arrays.

Adam thanks for the review. I don't have anything specific to check. I was required to breakout the functions into multiple files because the file was over 1000 lines long which throws a pylint error.

[akeeste]

@ssolson That makes sense. I'll assume that any other line changes were similar to those done in other loads submodules. Since the tests are passing, I'm assuming all those changes are okay.

The only outstanding thing here is to decide if we can consolidate type handling functions. Let me know what you think after reviewing #285. I support merging both this and #285, then updating/standardizing type handling in the loads module in a new small PR.

[ssolson]

@ssolson That makes sense. I'll assume that any other line changes were similar to those done in other loads submodules. Since the tests are passing, I'm assuming all those changes are okay.

The only outstanding thing here is to decide if we can consolidate type handling functions. Let me know what you think after reviewing #285. I support merging both this and #285, then updating/standardizing type handling in the loads module in a new small PR.

@akeeste I think there are a couple of things to address on #285 so lets get this PR merged into develop and I think you should move your 2 util functions into type_handling vs the current data_utils as discussed in my review.

[akeeste]

@ssolson agreed, type_handling is a better name. Merging!