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

Update cbsodata3.py #11

Merged
merged 4 commits into from
Sep 5, 2019
Merged

Update cbsodata3.py #11

merged 4 commits into from
Sep 5, 2019

Conversation

ncvanegmond
Copy link
Contributor

added positional arguments for functions get_table_list and _download_metadata to allow for use of proxies

added positional arguments for functions get_table_list and _download_metadata to allow for use of proxies
Copy link
Owner

@J535D165 J535D165 left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! Interesting feature.

A couple of remarks before merging.

  1. Can you add this option for the other functions (like get_data, get_info, ... ) as well?
  2. Add proxies to Options class. Add self.proxies = None here
    self.use_https = True
    self.api_version = "3"
    Use the option manager within the functions to get the global option. See example below.
def get_info(table_id, catalog_url=None, proxies=None):
    """Get information about a table.
    Parameters
    ----------
    table_id : str
        The identifier of the table.
    catalog_url : str
        The url of the catalog. Default "opendata.cbs.nl".
    Returns
    -------
    dict
        Table information
    """

    _proxies = options.proxies if proxies is None else proxies

    info_list = _download_metadata(
        table_id,
        "TableInfos",
        catalog_url=_get_catalog_url(catalog_url),
        proxies = proxies
    )

    if len(info_list) > 0:
        return info_list[0]
    else:
        return None

Usage:

import cbsodata

cbsodata.options.proxies = "MY PROXIES"

cbsodata.get_info("82070ENG")

which is the same as

import cbsodata

cbsodata.get_info("82070ENG", proxies = "MY PROXIES")

cbsodata/cbsodata3.py Show resolved Hide resolved
* Added proxies to Options class
* Added option manager within the relevant functions to get the global option
* Change the following functions: get_table_list, get_info, get_meta, get_data
> Added proxies as positional var 
> Added option manager
> Added Docstring (definition quoted from requests lib)
@J535D165
Copy link
Owner

J535D165 commented Sep 5, 2019

Great implementation.

The tests fail due to style errors https://travis-ci.org/J535D165/cbsodata/jobs/581151465. Thereafter, I can merge.

Would you like to have a new release on the short term?

(FYI @jolienoomens)

Fixed style errors
fixed two additional style errors that did not show up in spyder's real-code code style analysis
@ncvanegmond
Copy link
Contributor Author

Thanks for considering the PR, and all your feedback and patience. First time making a pull requests, so figuring it out as I go :) Think I got all errors this time around.

If you would be able to do a new release on the short-term, that would be much appreciated! Very helpful library, even more so now we can use it behind a proxy ;)

@J535D165 J535D165 merged commit 7ed6552 into J535D165:master Sep 5, 2019
@J535D165
Copy link
Owner

J535D165 commented Sep 5, 2019

Thanks, merged and a new release on PyPI. Good work for your first PR!

Note. Add support for proxies in odata4 #8.

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