Skip to content
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

Added datasource Bank of England database #85

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

dwinrow
Copy link
Contributor

@dwinrow dwinrow commented Apr 6, 2022

No description provided.

@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #85 (cbaa9a4) into master (cbeb769) will decrease coverage by 2.85%.
The diff coverage is 84.61%.

❗ Current head cbaa9a4 differs from pull request most recent head f49adfa. Consider uploading reports for the commit f49adfa to get more accurate results

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   93.10%   90.24%   -2.86%     
==========================================
  Files           4        4              
  Lines          58       82      +24     
==========================================
+ Hits           54       74      +20     
- Misses          4        8       +4     
Impacted Files Coverage Δ
src/MarketData.jl 100.00% <ø> (ø)
src/downloads.jl 89.47% <84.61%> (-2.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbeb769...f49adfa. Read the comment docs.

:VPD => "Y"
```
"""
struct BoeOpt <: AbstractQueryOpt
Copy link
Member

@Arkoniak Arkoniak Apr 19, 2022

Choose a reason for hiding this comment

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

Usually we use CamelCase for modules and type names, not for fields.
Can we switch to lowercase? Like

struct BoeOpt
    date_from::Date
    date_to::Date
    using_codes::Bool
    csvf::String
    vpd::Bool
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An AbstractQueryOpt is passed to the HTTP GET command with its fieldnames and values as a query.

I chose fieldnames to be consistent with the API documentation. https://www.bankofengland.co.uk/boeapps/database/Help.asp#CSV

However, I agree that lowercase is standard for fieldnames, and since the parameter names in a HTTP GET query are case-insensitive this shouldn't cause an issue. However, underscores would not be consistent with AbstractQueryOpt as I would need to change the field names to construct the query.

@Arkoniak
Copy link
Member

There was an error

BOE: Test Failed at /home/travis/build/JuliaQuant/MarketData.jl/test/downloads.jl:31
Expression: boe("asdfasdf")
Expected: APIError
Thrown: UndefVarError

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants