-
Notifications
You must be signed in to change notification settings - Fork 9
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
Feature refactor #110
Feature refactor #110
Conversation
# Conflicts: # test/data.py
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've reviewed up until the provider plugin files.
@@ -150,9 +150,9 @@ | |||
" Returns quest uid for merged raster\n", | |||
" \"\"\"\n", | |||
" \n", | |||
" features = quest.api.get_features(service_uri, filters={'bbox': bbox})\n", | |||
" features = quest.api.search_catalog(service_uri, filters={'bbox': bbox})\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.
Let's change the variable name here from features
to entries
or something like that.
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 in the notebooks and will assume that you took care of this one. You mentioned in our reviewal process that you would. Let me know if you haven't. Thanks
quest/api/catalog.py
Outdated
|
||
uris = [] | ||
for _, data in entries.iterrows(): | ||
uri = new_dataset(collection=collection, catalog_entry=data.to_frame().transpose(), source=DatasetSource.WEB_SERVICE) |
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 think we need to check to see if the cataloge_entries are from quest and if so make the source derived.
quest/api/catalog.py
Outdated
i.e. point/line/polygon | ||
parameter (string, optional): filter catalog_entries by parameter | ||
bbox (string, optional): filter catalog_entries by bounding box | ||
|
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.
We need to update this list of filters in the docstring to include all of the filters that are handled in the code.
quest/api/datasets.py
Outdated
@@ -32,25 +31,25 @@ def download(feature, file_path, dataset=None, **kwargs): | |||
details of downloaded data | |||
|
|||
""" | |||
service_uri = feature | |||
service_uri = catalog_entry | |||
if not service_uri.startswith('svc://'): |
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.
Delete this section. We can now assume that service_uri
will always start with 'svc://'.
quest/api/datasets.py
Outdated
df = df['service'] + '/' + df['service_id'] | ||
service_uri = df.tolist()[0] | ||
|
||
if file_path is None: | ||
pass | ||
|
||
provider, service, feature = parse_service_uri(service_uri) | ||
provider, service, catalog_entry = parse_service_uri(service_uri) |
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.
change catalog_entry
to catalog_id
quest/plugins/user_provider.py
Outdated
fmt = self.features_format | ||
paths = self._get_paths(self.features_file) | ||
def search_catalog(self, **kwargs): | ||
fmt = self.catalog_entries_format |
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.
self.catalog_file_format
quest/plugins/user_provider.py
Outdated
paths = self._get_paths(self.features_file) | ||
def search_catalog(self, **kwargs): | ||
fmt = self.catalog_entries_format | ||
paths = self._get_paths(self.catalog_entries_file) |
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.
self.catalog_file
quest/plugins/user_provider.py
Outdated
polys.append(Feature(geometry=util.bbox2poly(x1, y1, x2, y2, as_geojson=True), properties=properties, id=feature_id)) | ||
features = FeatureCollection(polys) | ||
features = util.to_geodataframe(features) | ||
catalog_entry_id, x1, y1, x2, y2 = line.split() |
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.
change catalog_entry_id
to catalog_id
quest/util/misc.py
Outdated
"""Converts a list of uris into a pandas dataframe. | ||
|
||
Notes: | ||
Classified by resource type. | ||
|
||
Args: | ||
uris (list or string): List of Quest uris to classify into the following types: 'collections', 'services', | ||
'publishers', 'datasets', or 'features'. | ||
'publishers', 'datasets', or 'catalog_ids*'. |
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.
remove catalog_ids*
quest/util/misc.py
Outdated
@@ -371,19 +373,16 @@ def uuid(resource_type): | |||
|
|||
Notes: | |||
First character of uuid is replaced with 'f' or 'd' respectively |
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.
First character of uuid is replaced with 'd' for resource_type
dataset.
from .features import ( | ||
add_features, | ||
get_features, | ||
from .catalog import ( |
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.
E402 module level import not at top of file
|
||
@add_async | ||
def search_catalog(uris=None, expand=False, as_dataframe=False, as_geojson=False, | ||
update_cache=False, filters=None, queries=None): |
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.
E128 continuation line under-indented for visual indent
Note: nested dicts are parsed out as a multi-index tag where keys for nested dicts are joined with ':'. | ||
""" | ||
# group uris by type | ||
grouped_uris = util.classify_uris(service_uris, as_dataframe=False, exclude=['collections', 'catalog_entries', 'datasets']) |
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.
E501 line too long (127 > 120 characters)
|
||
Args: | ||
geometry (string or Shapely.geometry.shape, optional, Default=None): | ||
well-known-text or Shapely shape representing the geometry of the catalog_entry. Alternatively `geom_type` and `geom_coords` can be passed. |
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.
E501 line too long (151 > 120 characters)
|
||
if isinstance(parameters,pd.DataFrame) and feature: | ||
idx = parameters['service_id'] == feature | ||
if isinstance(parameters,pd.DataFrame) and catalog_id: |
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.
E231 missing whitespace after ','
'service_id': catalog_id, | ||
'geometry': geometry, | ||
'metadata': metadata, | ||
} |
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.
E123 closing bracket does not match indentation of opening bracket's line
from .datasets import get_datasets | ||
from quest.database.database import get_db, db_session | ||
from quest.database.database import get_db, db_session, select_datasets | ||
from ..static import DatasetStatus |
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.
F401 '..static.DatasetStatus' imported but unused
import pytest | ||
|
||
from data import SERVICES_FEATURE_COUNT, CACHED_SERVICES | ||
from quest.static import DatasetStatus |
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.
F401 'quest.static.DatasetStatus' imported but unused
pytestmark = pytest.mark.usefixtures('reset_projects_dir', 'set_active_project') | ||
|
||
SERVICE_URIS = [ | ||
'svc://usgs-ned:19-arc-second/581d2561e4b08da350d5a3b2', |
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.
E126 continuation line over-indented for hanging indent
'svc://usgs-ned:19-arc-second/581d2561e4b08da350d5a3b2', | ||
'svc://noaa-ncdc:gsod/028140-99999', | ||
'svc://usgs-nwis:iv/01529950', | ||
] |
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.
E123 closing bracket does not match indentation of opening bracket's line
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.
There are some stickler issues still, but I'll fix those with my PR once this is merged in.
No description provided.