Changes to documentation & core.py

lewis-morris commented 2 years ago

Updates

core.py so it will accept path parameters descriptions. This is added to the view's docstring. It's a bit hacky and I'm sure you can think of a better way of doing this. But I wanted the feature.
core.py also now reads the docstring from custom decorators so you can append the "description" of that endpoint with extra information from each decorator i.e extra info from @paginated_response
documentation addition, you can also feel free to change if you want, but others could benefit from this.

So many commits to the docs but I've not really used rst before or sphynx before so had to play about with it.

miguelgrinberg commented 2 years ago

Hey, thanks for the updates. I have to review the changes you've made in core.py, but the docs improvements are a great addition that I definitely can use.

Note that I will not be able to merge all these things that you have changed together, as I prefer to not batch several changes in a single commit, as that makes reviewing the code in the future much harder.

miguelgrinberg commented 2 years ago

Okay, here is some feedback:

For documenting path parameters, I do like the idea of doing it, but creating a custom format within the docstring to do this is not great, especially because there are already standards for this, like the format that Sphinx uses. I was thinking about this, and came up with the idea of using annotations for this, which works well as long as the project doesn't use annotations for type hints. Example:
```
@users.route('/users/<int:id>', methods=['GET'])
@authenticate(token_auth)
@response(user_schema)
@other_responses({404: 'User not found'})
def get(id: 'The id of the user'):
    """Retrieve a user by id"""
    return db.session.get(User, id) or abort(404)
```
Appending comments from decorators is a strange idea, because you really have no way to know which decorators you may want to take docs from or which you wouldn't. If I understand your change correctly you are only looking at decorators that wrap the arguments decorator from this package, and only if the query location is used. Any decorators that do something outside of query string arguments would be ignored. So I'm going to pass on this change, as I don't see how it can work in a more generic way.

So basically what I'm going to do is commit the annotation change separately, and from this PR I'm going to use the new documentation page that you created and complete it with the remaining documentation options that you have not included.

Thanks!

lewis-morris commented 2 years ago

Okay, here is some feedback:
For documenting path parameters, I do like the idea of doing it, but creating a custom format within the docstring to do this is not great, especially because there are already standards for this, like the format that Sphinx uses. I was thinking about this, and came up with the idea of using annotations for this, which works well as long as the project doesn't use annotations for type hints. Example:
@users.route('/users/<int:id>', methods=['GET'])
@authenticate(token_auth)
@response(user_schema)
@other_responses({404: 'User not found'})
def get(id: 'The id of the user'):
  """Retrieve a user by id"""
  return db.session.get(User, id) or abort(404)
Appending comments from decorators is a strange idea, because you really have no way to know which decorators you may want to take docs from or which you wouldn't. If I understand your change correctly you are only looking at decorators that wrap the arguments decorator from this package, and only if the query location is used. Any decorators that do something outside of query string arguments would be ignored. So I'm going to pass on this change, as I don't see how it can work in a more generic way.
So basically what I'm going to do is commit the annotation change separately, and from this PR I'm going to use the new documentation page that you created and complete it with the remaining documentation options that you have not included.

Thanks!

Anything you use is fine by me.

lewis-morris commented 2 years ago

Okay, here is some feedback:
For documenting path parameters, I do like the idea of doing it, but creating a custom format within the docstring to do this is not great, especially because there are already standards for this, like the format that Sphinx uses. I was thinking about this, and came up with the idea of using annotations for this, which works well as long as the project doesn't use annotations for type hints. Example:
@users.route('/users/<int:id>', methods=['GET'])
@authenticate(token_auth)
@response(user_schema)
@other_responses({404: 'User not found'})
def get(id: 'The id of the user'):
  """Retrieve a user by id"""
  return db.session.get(User, id) or abort(404)
Appending comments from decorators is a strange idea, because you really have no way to know which decorators you may want to take docs from or which you wouldn't. If I understand your change correctly you are only looking at decorators that wrap the arguments decorator from this package, and only if the query location is used. Any decorators that do something outside of query string arguments would be ignored. So I'm going to pass on this change, as I don't see how it can work in a more generic way.
So basically what I'm going to do is commit the annotation change separately, and from this PR I'm going to use the new documentation page that you created and complete it with the remaining documentation options that you have not included.

Thanks!

The idea of adding documentation to the decorator was that you could add, for instance, a "filter" decorator that accepts any schema you like, but having generic instructions on its use that would relate to all schemas.

Maybe It only fits my use case, but it's working for me.

miguelgrinberg commented 2 years ago

Added new documentation here: https://apifairy.readthedocs.io/en/latest/guide.html.

Thanks for your help!

miguelgrinberg / APIFairy

Changes to documentation & core.py #52

Updates