Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

20 lookup table class #25

Merged
merged 13 commits into from
Feb 22, 2024

Conversation

alberto-escobar
Copy link
Contributor

Description

Verification

  • Created testing suite in lut_test, and has passing results when running tests

Resources

  • None

@alberto-escobar alberto-escobar changed the title User/alberto escobar/20 lookup table class 20 lookup table class Jan 27, 2024
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
Copy link
Contributor

@DFriend01 DFriend01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, just a few more comments on your LUT class and tests.

controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
controller/common/lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Outdated Show resolved Hide resolved
tests/unit/wingsail/common/test_lut.py Show resolved Hide resolved
@alberto-escobar
Copy link
Contributor Author

@DFriend01 All issues resolved, one question I have:
Are the failed tests going to be a problem?

@DFriend01
Copy link
Contributor

DFriend01 commented Feb 10, 2024

@DFriend01 All issues resolved, one question I have: Are the failed tests going to be a problem?

@alberto-escobar If the tests pass on your local machine, then this is probably an infrastructure problem. I'll look into it this weekend.

However, the flake8 test should be passing. Make sure that the Python autoformatter is enabled. You can also run the flake8 task in sailbot workspace to diagnose the problem on your end and resolve the lining errors yourself, or at least figure out where the problem is so you can apply the autoformatter there.

@alberto-escobar
Copy link
Contributor Author

alberto-escobar commented Feb 10, 2024

@DFriend01 All issues resolved, one question I have: Are the failed tests going to be a problem?

@alberto-escobar If the tests pass on your local machine, then this is probably an infrastructure problem. I'll look into it this weekend.

However, the flake8 test should be passing. Make sure that the Python autoformatter is enabled. You can also run the flake8 task in sailbot workspace to diagnose the problem on your end and resolve the lining errors yourself, or at least figure out where the problem is so you can apply the autoformatter there.

@DFriend01 I figured out the problem, in test_lut.py I added:

assert math.isclose(testLUT(50000), 5.75)

where ever there was an instance of a LUT object created but never used. This cleared up flake8 errors.

I also had to add the following logic to __linearInterpolation in lut.py to mitigate an error of np.interp returning an NDArray:

    def __linearInterpolation(self, x: Scalar) -> float:
        output = np.interp(x, self.x, self.y)
        if isinstance(output, np.ndarray):
            raise ValueError(
                "linear interpolation returned a NDArray when it should have returned a float"
            )
        return output

I made a unit test for this in test_lut called test_linear_interpolation_exception

Copy link
Contributor

@DFriend01 DFriend01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warnings seemed to have been resolved. Great work!

@alberto-escobar alberto-escobar merged commit 5252e3e into main Feb 22, 2024
12 checks passed
@alberto-escobar alberto-escobar deleted the user/alberto-escobar/20-lookup-table-class branch February 22, 2024 01:35
@DFriend01 DFriend01 mentioned this pull request Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lookup Table Class
2 participants