-
Notifications
You must be signed in to change notification settings - Fork 448
Cleaning up samples #47
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
Conversation
| for connection in sample_workbook.connections]) | ||
|
|
||
| # Update tags and show_tabs flag | ||
| original_tag_set = set(sample_workbook.tags) |
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 is a simpler way to copy a set and doesn't require copy.copy
samples/explore_datasource.py
Outdated
| import tableauserverclient as TSC | ||
|
|
||
|
|
||
| parser = argparse.ArgumentParser(description='Explore datasource functions supported by the Server API.') |
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.
Should probably move these into the main conditional also
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.
I debated that too -- I wonder if I should wrap everything in a main(), and then just have:
if __name__ == "__main__":
main()
So it's easier to tell runtime from any setup?
| # Query projects for use when demonstrating publishing and updating | ||
| all_projects, pagination_item = server.projects.get() | ||
| default_project = next((project for project in all_projects if project.is_default()), None) | ||
| # SIGN IN |
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.
I'm not sure if this is just a product of github, but the spaces here are showing different than then ones below. Make sure they are all spaces, not a mix of tabs and spaces.
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.
Yeah I noticed that when it posted, weird, I'll make sure they're all spaces.
Though not required, I moved all of the samples code to be wrapped in
if __name__ == "__main__"blocks since that's best practice for command line scripts.I also cleaned up imports to match pep8, and removed one or two that were unused.
I think we can refactor some of the really long lines a bit more to make them more readable -- especially with respect to the Filter/Sort options, but that can come in a later checkin.