-
Notifications
You must be signed in to change notification settings - Fork 0
Updated test file and models.py #2
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
| from datetime import datetime | ||
|
|
||
| from pydantic import field_validator, model_validator | ||
| from pydantic import field_validator, model_validator, validator |
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.
field_validator and validator are unused. Please remove the imports
| from pydantic import field_validator, model_validator | ||
| from pydantic import field_validator, model_validator, validator | ||
| from sqlmodel import Field, SQLModel | ||
| from sqlalchemy import JSON |
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.
unused. please remove the import
| import ast | ||
| from fastapi import FastAPI | ||
| from typing import List | ||
| from typing import List, Optional |
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.
unused ...
app/versions/v1/models.py
Outdated
| from typing import List, Optional | ||
|
|
||
|
|
||
| app = FastAPI(version="1") |
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.
unused and this is declared in app.py not here. Why is this needed?
| "authors_firstname": data_request.authorFNames, | ||
| "authors_lastname": data_request.authorLNames, | ||
| # date | ||
| "start_datetime": data_request.start_date, |
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.
STAC items can have a single date property called datetime if it just represents a single point in time. We should check whether start_datetime == end_datetime and if so set the datetime property only.
Also, datetime can possibly be null in which case we'd want to handle that option as well.
| #STAC item time (creation time) | ||
| datetime_utc = datetime.now(tz=timezone.utc) | ||
| #Create list of custom item properties (other than geometry and time), which will be added to the STAC item | ||
| item_properties = { |
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 should try to use existing stac extensions as much as possible when we can. So for example, for the author information we can use the contacts extension.
| "variables": data_request.variables, | ||
| # models | ||
| "models": data_request.models, | ||
| "item_links": [{ |
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.
links should be outside the properties key. (links and properties are siblings, not nested).
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.
Also, please include type in all links when possible.
| "models": data_request.models, | ||
| "item_links": [{ | ||
| "rel": "self", | ||
| "href": data_request.path |
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 bit tricky because this will be the path to the file in the users directory but the actual stac object should be the location of the file in the THREDDS catalog (or however the file is being served publicly). I don't really know what the proper solution is to automatically generate that yet but for now can you add a comment that we need to figure this out.
| }, | ||
| { | ||
| "rel": "derived_from", | ||
| "href": data_request.input |
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.
If there are multiple of these we should create multiple links
| }, | ||
| { | ||
| "rel": "linked_files", | ||
| "href": data_request.link |
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.
Same here. We should create multiple links if there are multiples
| "href": data_request.path | ||
| }, | ||
| { | ||
| "rel": "derived_from", |
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 should use established rel values when possible. Are derived_from and linked_files standard names or are they custom to this?
| if data_request.geometry == "Point": | ||
| myfile = { | ||
| "type": "Point", | ||
| "coordinates": [data_request.latitude, data_request.longitude] |
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 this be latitude and longitude or do you want the serialized version (not extracted by json.loads)?
| shape = gbbox.Point(**myfile) | ||
| geobbox = shape.bbox() | ||
| if data_request.geometry == "Line": | ||
| geom = shp(myfile) |
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.
Just curious... does gbbox not support Lines?
Closes #1