-
-
Notifications
You must be signed in to change notification settings - Fork 306
fix support for custom formatters (#1997) #2144
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
base: master
Are you sure you want to change the base?
Conversation
dbbce65 to
b364419
Compare
b364419 to
051ccbb
Compare
051ccbb to
4ae51b6
Compare
webb-ben
left a comment
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.
Looks good to me. I will test later today. Some small questions / comments.
-
This implements custom formatters for OAFeat and OARec GeoJSON responses. Can we add some additional documentation to clarify this? Is support for EDR CovJSON Formatters an entirely different issue? I have a simple Point-based CovJSON -> CSV formatter that I would like to synchronize these changes with.
-
After this is merged I will migrate our OGR Formatter to https://github.com/cgs-earth/pygeoapi-plugins. I will want to figure out how much of this can be offloaded to the configuration options introduced. Depending on the path this takes, a general OGR formatter could subsume #2136 and our custom formatter.
| self.mimetype = None | ||
| self.geom = False | ||
|
|
||
| self.name = formatter_def['name'] |
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 in a try/catch?
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.
For some reason this is formatted to the wrong line. In the base provider, all required fields are in a try/catch. Should
self.name = formatter_def['name']
follow suit?
| super().__init__({'name': 'cooljson', 'geom': None}) | ||
| self.mimetype = 'application/json; subtype:mycooljson' | ||
| self.f = 'cooljson' # f= value |
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.
Does the formatter need to response both f= and the Accept Header?
| formatter = load_plugin('formatter', | ||
| {'name': 'CSV', 'geom': True}) | ||
| elif request.format in [df.f for df in dataset_formatters.values()]: | ||
| formatter = [v for k, v in dataset_formatters.items() if |
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.
| formatter = [v for k, v in dataset_formatters.items() if | |
| formatter = [v for v in dataset_formatters.values() if |
Are we using the key in this?
Overview
This PR updates formatters support to allow for custom formatters for item types.
Related Issue / discussion
#1997
Additional information
Dependency policy (RFC2)
Updates to public demo
Contributions and licensing
(as per https://github.com/geopython/pygeoapi/blob/master/CONTRIBUTING.md#contributions-and-licensing)