-
Notifications
You must be signed in to change notification settings - Fork 0
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
get_all_tables()
#185
base: main
Are you sure you want to change the base?
get_all_tables()
#185
Conversation
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'd like to discuss what these routines are for before signing off on the call interfaces.
You can limit to the columns of one table by passing the `table` | ||
parameter. By default the `<table_name>` is included, but this can be | ||
removed setting `include_table=False`. | ||
|
||
If `include_schema=True` return all columns of the db in | ||
<schema>.<table_name>.<column_name> format. Note this will essentially | ||
duplicate the output, as the working and production schemas have the |
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 isn't quite accurate. It assumes 1) the connection was made using a namespace and 2) dialect is not SQLite.
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.
Tried to make this more clear in the doc string
src/dataregistry/query.py
Outdated
|
||
return table_list | ||
|
||
def get_all_columns(self, table=None, include_table=True, include_schema=False): |
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.
Is table=None
the most useful default? It depends on who uses it, I suppose. For typical users table="dataset"
might be better.
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.
Defaulted it to the dataset table, added a note in the docstring that "None" will get back all tables
Added a
get_all_tables()
function toQuery
class to return a set of the table names in the database.Also updated
get_all_columns()
to be able to just return the column names from a chosen table, as well as the default behaviour of all tables.