search

openclimatefix / uk-pv-forecast-blend

Service to blend forecast together
0 stars 0 forks source link

if Null, make properties an empty dict #28

Closed aryanbhosale closed 3 months ago

aryanbhosale commented 3 months ago

Pull Request

Description

Made the properties an empty dictionary if found to be null

Fixes #

18

Checklist:

aryanbhosale commented 3 months ago

Hi @peterdudfield , so sorry for the delay, I was really busy prepping for my quizzes that i had today and have for the next 2 days:( But ive made the changes and tested it, it doesnt give the same error it did last time and i think we can try running tests here could you please review it and let me know if this is what was expected? thank you :)

peterdudfield commented 3 months ago

Hi @peterdudfield , so sorry for the delay, I was really busy prepping for my quizzes that i had today and have for the next 2 days:( But ive made the changes and tested it, it doesnt give the same error it did last time and i think we can try running tests here could you please review it and let me know if this is what was expected? thank you :)

Ive changed the branch to development, as last time the tests failed on main.

Have you run the test locally? Do they work?

aryanbhosale commented 3 months ago

Hi @peterdudfield , so sorry for the delay, I was really busy prepping for my quizzes that i had today and have for the next 2 days:( But ive made the changes and tested it, it doesnt give the same error it did last time and i think we can try running tests here could you please review it and let me know if this is what was expected? thank you :)

Ive changed the branch to development, as last time the tests failed on main.

Have you run the test locally? Do they work?

Ive made the changes , but im running into these errors: image image image

aryanbhosale commented 3 months ago

Hi @peterdudfield , so sorry for the delay, I was really busy prepping for my quizzes that i had today and have for the next 2 days:( But ive made the changes and tested it, it doesnt give the same error it did last time and i think we can try running tests here could you please review it and let me know if this is what was expected? thank you :)

Ive changed the branch to development, as last time the tests failed on main. Have you run the test locally? Do they work?

I ran export PYTHONPATH=${PYTHONPATH}:./forecast_blend first and then pytest now i see this error: image

aryanbhosale commented 3 months ago

Hi @peterdudfield , so sorry for the delay, I was really busy prepping for my quizzes that i had today and have for the next 2 days:( But ive made the changes and tested it, it doesnt give the same error it did last time and i think we can try running tests here could you please review it and let me know if this is what was expected? thank you :)

Ive changed the branch to development, as last time the tests failed on main. Have you run the test locally? Do they work?

I ran export PYTHONPATH=${PYTHONPATH}:./forecast_blend first and then pytest now i see this error: image

Hi @peterdudfield , ive fixed this in my latest commit, i think this can be merged, could you please review it? I've run the tests locally and all 22 of them pass as expected, I think we can change the merge head to main

peterdudfield commented 3 months ago

Excellent, looks, good. Could you remove all the pycache files and then ill merge?

aryanbhosale commented 3 months ago

Excellent, looks, good. Could you remove all the pycache files and then ill merge?

Done in my latest commit!

aryanbhosale commented 3 months ago

i think we can merge https://github.com/openclimatefix/uk-pv-forecast-blend/tree/development into main since the tests pass!