Test B11 Implementation

OpenEnergyDashboard / OED

Open Energy Dashboard (OED)

Mozilla Public License 2.0

82 stars 307 forks source link

Test B11 Implementation #1080

Closed ArtP1 closed 11 months ago

ArtP1 commented 1 year ago

Description

The test verifies that the API can correctly return 1 day bars with 15 minute reading intervals for quantity units with +-inf start/end time. The test also verifies that the API can correctly convert kWh to BTU using the provided conversion factor.

Co-Authored by: @TamNganLY, and @Zia-wang

Implements B11 test for testing readings #962

Type of change

[ ] Note merging this changes the database configuration.
[x] This change requires a documentation update

Checklist

[x] I have followed the OED pull request ideas
[x] I have removed text in ( ) from the issue request
[x] You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

huss commented 12 months ago

@ArtP1 This is marked WIP and I don't normally review these unless asked. I just wanted to verify this is still under development so I should not be doing anything with it.

ArtP1 commented 12 months ago

@ArtP1 This is marked WIP and I don't normally review these unless asked. I just wanted to verify this is still under development so I should not be doing anything with it.

Hey @huss , thanks for checking in, yes this PR is still work in progress.

Zia-wang commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

huss commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

@Zia-wang Thanks for reporting this. I think you are correct since the value for line tests is the one you note. I think I forgot to change the bar ones when I found this before. However, I'm confused about putting this into the code. This test does a chained conversion so this value should have been automatically calculated and that is what I see in your PR code. I tried the correct version in the spreadsheet to generate the bar values and got the same expected file as in the DesignDocs. I also see very similar values in your expected file in the PR. I see it is different in some lesser significant digits from what was already committed and getting a merge conflict. Thus, I want to be sure I understand what you did and what was changed. Sorry for the confusion.

Zia-wang commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

@Zia-wang Thanks for reporting this. I think you are correct since the value for line tests is the one you note. I think I forgot to change the bar ones when I found this before. However, I'm confused about putting this into the code. This test does a chained conversion so this value should have been automatically calculated and that is what I see in your PR code. I tried the correct version in the spreadsheet to generate the bar values and got the same expected file as in the DesignDocs. I also see very similar values in your expected file in the PR. I see it is different in some lesser significant digits from what was already committed and getting a merge conflict. Thus, I want to be sure I understand what you did and what was changed. Sorry for the confusion.

Thanks for your clarification. Yeah, I meant to say we put 3414.142 into the Excel spreadsheet and got the same values as the expected file, not into the code. We didn't change any other values in the codebase. Sorry about the confusion! We just wanted to bring up this discrepancy we noted in the documentation. It's all good as long as it is known. Thank you again.

huss commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

@Zia-wang Thanks for reporting this. I think you are correct since the value for line tests is the one you note. I think I forgot to change the bar ones when I found this before. However, I'm confused about putting this into the code. This test does a chained conversion so this value should have been automatically calculated and that is what I see in your PR code. I tried the correct version in the spreadsheet to generate the bar values and got the same expected file as in the DesignDocs. I also see very similar values in your expected file in the PR. I see it is different in some lesser significant digits from what was already committed and getting a merge conflict. Thus, I want to be sure I understand what you did and what was changed. Sorry for the confusion.

Thanks for your clarification. Yeah, I meant to say we put 3414.142 into the Excel spreadsheet and got the same values as the expected file, not into the code. We didn't change any other values in the codebase. Sorry about the confusion! We just wanted to bring up this discrepancy we noted in the documentation. It's all good as long as it is known. Thank you again.

Sorry but I'm still confused. When I put 3412.08 into the expect spreadsheet I get the same values as in the expected CSV file in DesignDocs. When I use 3414.142 I get different values. I think you are saying that 3414.142 gives you the same values. Is that correct? Your first message made me think it was the other value. If it what is in your last comment then I need to think about this some more as that was not what I think I get.

Zia-wang commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

@Zia-wang Thanks for reporting this. I think you are correct since the value for line tests is the one you note. I think I forgot to change the bar ones when I found this before. However, I'm confused about putting this into the code. This test does a chained conversion so this value should have been automatically calculated and that is what I see in your PR code. I tried the correct version in the spreadsheet to generate the bar values and got the same expected file as in the DesignDocs. I also see very similar values in your expected file in the PR. I see it is different in some lesser significant digits from what was already committed and getting a merge conflict. Thus, I want to be sure I understand what you did and what was changed. Sorry for the confusion.

Thanks for your clarification. Yeah, I meant to say we put 3414.142 into the Excel spreadsheet and got the same values as the expected file, not into the code. We didn't change any other values in the codebase. Sorry about the confusion! We just wanted to bring up this discrepancy we noted in the documentation. It's all good as long as it is known. Thank you again.

Sorry but I'm still confused. When I put 3412.08 into the expect spreadsheet I get the same values as in the expected CSV file in DesignDocs. When I use 3414.142 I get different values. I think you are saying that 3414.142 gives you the same values. Is that correct? Your first message made me think it was the other value. If it what is in your last comment then I need to think about this some more as that was not what I think I get.

3412.08 is the result we got from our calculation and when we put this value into the spreadsheet, we had the same values as in the expected CSV file in DesignDocs. 3414.142 is the provided value in the table and we thought this was a wrong value.

Zia-wang commented 12 months ago

Hi @huss, we found an issue with the given expected value of the slope in the testing documentation. The given value of the slope of B11 is 3414.142, but we calculated the slope manually and got 3412.08. When we put 3412.08 in our test code, we had the same values as the expected readings file and all checks passed.

@Zia-wang Thanks for reporting this. I think you are correct since the value for line tests is the one you note. I think I forgot to change the bar ones when I found this before. However, I'm confused about putting this into the code. This test does a chained conversion so this value should have been automatically calculated and that is what I see in your PR code. I tried the correct version in the spreadsheet to generate the bar values and got the same expected file as in the DesignDocs. I also see very similar values in your expected file in the PR. I see it is different in some lesser significant digits from what was already committed and getting a merge conflict. Thus, I want to be sure I understand what you did and what was changed. Sorry for the confusion.

Thanks for your clarification. Yeah, I meant to say we put 3414.142 into the Excel spreadsheet and got the same values as the expected file, not into the code. We didn't change any other values in the codebase. Sorry about the confusion! We just wanted to bring up this discrepancy we noted in the documentation. It's all good as long as it is known. Thank you again.

Sorry but I'm still confused. When I put 3412.08 into the expect spreadsheet I get the same values as in the expected CSV file in DesignDocs. When I use 3414.142 I get different values. I think you are saying that 3414.142 gives you the same values. Is that correct? Your first message made me think it was the other value. If it what is in your last comment then I need to think about this some more as that was not what I think I get.

3412.08 is the result we got from our calculation and when we put this value into the spreadsheet, we had the same values as in the expected CSV file in DesignDocs. 3414.142 is the provided value in the table and we thought this was a wrong value.

Here is our calculation by @TamNganLY 370062347_909348913992537_3216044131470900570_n

huss commented 11 months ago

The version of the expected file differed from the DesignDoc by numerical roundoff and formatting (zero at end). I changed to the DesignDoc one to be consistent. This led to a merge conflict that was resolved by merging in development. The conflict was not actual but due to the way the changes were done.