Skip to content

Conversation

@rwb27
Copy link
Contributor

@rwb27 rwb27 commented Jul 6, 2021

The OpenAPI docs currently leave quite a lot to be desired in the OpenFlexure Microscope. In particular, using the same "description" for GET and POST methods on an Action is confusing, I think. I've changed the APISpec plugin that extracts descriptions so that it looks on the get and post methods first for a docstring, then falls back to the class.

This enables me to make a much nicer API description. However, it might change descriptions in existing code, if there are docstrings on the methods that are currently being ignored. I think this is likely a minor enough problem that it can be ignored...

I have also made what I think is quite a nice factory function to remove boilerplate code from ActionView definitions, I'm happy to add that here, or submit another PR.

rwb27 added 5 commits July 1, 2021 15:06
Static views created with `static_from` had no metadata in the OpenAPI
representation.  This is fixed by subclassing our custom View.
There was a circular import problem, which I
have fixed by removing the `import builder` from `__init__.py`
This wasn't used, and only appeared in `__all__`.
It appears that removing the import has changed nothing;
`from labthings.views import *` still imports the `builder` module.
get_docstring used to discard all indentation.
It now uses `inspect.cleandoc()` to
remove spurious indents but preserve
intentional ones, which results in more nicely formatted
markdown docstrings.

I have not changed the default remove_newlines=True but I am quite
tempted to do so.

I'm not convinced by the current trailing space if remove_newlines
is True, but have not changed this
behaviour because there's no advantage
to doing so...
Previously, GET and POST requests both used the same description, which
doesn't make sense when they do different things.  I've changed this to
use` __doc__` or `description` attributes
 on the `get` or `post` method if available.
@jtc42
Copy link
Member

jtc42 commented Jul 6, 2021

The OpenAPI docs currently leave quite a lot to be desired in the OpenFlexure Microscope. In particular, using the same "description" for GET and POST methods on an Action is confusing, I think. I've changed the APISpec plugin that extracts descriptions so that it looks on the get and post methods first for a docstring, then falls back to the class.

Agreed, this is much nicer

This enables me to make a much nicer API description. However, it might change descriptions in existing code, if there are docstrings on the methods that are currently being ignored. I think this is likely a minor enough problem that it can be ignored...

I 100% have no problem with ignoring this. Nobody should be using the descriptions for anything other than human reading so it shouldn't break anything.

I have also made what I think is quite a nice factory function to remove boilerplate code from ActionView definitions, I'm happy to add that here, or submit another PR.

I think a separate PR would be better just for the sake of historical record.

@codecov
Copy link

codecov bot commented Jul 6, 2021

Codecov Report

Merging #243 (8ddfaec) into master (bdfbcba) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
- Coverage   90.54%   90.43%   -0.11%     
==========================================
  Files          40       40              
  Lines        1745     1746       +1     
  Branches      277      277              
==========================================
- Hits         1580     1579       -1     
  Misses        110      110              
- Partials       55       57       +2     
Flag Coverage Δ
unittests 90.43% <100.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/labthings/views/__init__.py 81.15% <ø> (-0.14%) ⬇️
src/labthings/apispec/plugins.py 89.33% <100.00%> (-1.21%) ⬇️
src/labthings/utilities.py 96.74% <100.00%> (+0.02%) ⬆️
src/labthings/views/builder.py 52.17% <100.00%> (ø)
src/labthings/extensions.py 85.07% <0.00%> (-0.75%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdfbcba...8ddfaec. Read the comment docs.

@jtc42 jtc42 merged commit 2b4c3c5 into master Jul 10, 2021
@jtc42
Copy link
Member

jtc42 commented Jul 10, 2021

I need to remember to tag a new release for this soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants