-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New functionality for UK Sold house price data #3969
New functionality for UK Sold house price data #3969
Conversation
…/OpenBBTerminal into feature/real_estate
Hi, is there something else required for me to do now? |
Sorry for the long delay. Will go through now. First thing I see is that in the menu:
Looks like you are missing the entries for the _town/_start etc in the i18 file. |
else: | ||
console.print("[red]Select valid postcode[/red]\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be here. This triggers the select valid postcode.
If i look for help :
The select valid postcode shouldnt appear if we have the -h flag. I pointed out where this happens in the code |
This happens for all other functions. |
These functions should have a limit in the views. For example, I run additionally, we probably want to specify a default start and end date (if I do |
default limit values added to model functions. |
I just applied a branch update and now I'm getting Linting errors in my controller that is a new file but was formatted ok prior to the branch update. Since the file is new and only in my pull request don't understand why it is failing now |
The black formatter published a new version last night, which is what gets installed on the linter. You can locally run |
Reviewpad failed with the error API rate limit exceeded for installation ID 30911239. |
Sorry for the delay. Was busy getting that last release fixed. If you fix the merge conflicts, I will give a final look today! |
Have resolved merge issues but now failing on some security checks I haven't seen before to do with Snyk. |
Something is still not correct with the poetry lock.The unit test check is failing on install dependencies.
|
I ran the poetry export commands and can't see any install errors but can see that some of not tests were skipped. Not sure how to resolve. |
ill take a stab at |
Everything was good on the update. Not sure what the snyk issues are, but I can ignore them for now. I just added a test script for our end and the rest looks good! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience!
No problem. I know you're busy. |
Description
![image](https://user-images.githubusercontent.com/4700433/212498707-6f352221-e9fe-42c3-908b-13ee7cef64bf.png)
] Screenshot of the feature or the bug before/after fix, if applicable. - [ ] Relevant motivation and context. - [ SPARQLWrapper] List any dependencies that are required for this change.How has this been tested?
Have ran all commands from application and also created a test class
Checklist:
Others