Skip to content

API resource output as class to utilize dot notation for attributes #513

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

Merged
merged 12 commits into from
Aug 22, 2022

Conversation

elamc-2
Copy link
Contributor

@elamc-2 elamc-2 commented Aug 7, 2022

Pull Request

What does this PR do?

Fixes #508

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ElamC for this PR ❤️
And well done you do a great job, the only feedback I have is my fault for not having detailed enough the outcome. On the naming, we decided with the team that all returns with results would be called such as: NameOfTheClassResults.
e.g: DocumentsResults, IndexesResults, TaskResults, KeysResults.
So sorry for the naming inconvenience.

@elamc-2
Copy link
Contributor Author

elamc-2 commented Aug 8, 2022

Thanks for the feedback @alallema. Just made a few changes

@elamc-2 elamc-2 requested a review from alallema August 8, 2022 12:58
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the modifications @ElamC 🔥 still a few changes 🚀

@elamc-2 elamc-2 requested a review from alallema August 12, 2022 17:27
@elamc-2
Copy link
Contributor Author

elamc-2 commented Aug 12, 2022

A note on the changes: document model allows for attributes to be accessed like document.title or document.poster. Class attributes are added based on keys from the dictionary that's passed in.

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the additions @ElamC 🚀 ✨ few last requests

Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last revision the rest is perfect 🚀 just a few typos all titles are in snake_case, I don't know why but it's just to keep some consistency between all files.

Thanks again @ElamC

@elamc-2 elamc-2 marked this pull request as draft August 18, 2022 07:31
@elamc-2 elamc-2 marked this pull request as ready for review August 19, 2022 12:56
@elamc-2 elamc-2 requested a review from alallema August 19, 2022 12:56
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much @ElamC for this PR ❤️ 🔥 🔥 🔥
LGTM!

@alallema
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 22, 2022

@bors bors bot merged commit 153c524 into meilisearch:main Aug 22, 2022
@alallema alallema added the breaking-change The related changes are breaking for the users label Sep 21, 2022
bors bot added a commit that referenced this pull request Sep 21, 2022
533: Update version for the next release (v0.20.0) r=brunoocasali a=alallema

Update to the next version

**Note:**
This release is a replacement for the v0.19.2 which should be a minor update instead of a patch update.
https://github.com/meilisearch/meilisearch-python/releases/tag/v0.19.2

## 💥 Breaking changes
* API resource output as class to utilize dot notation for attributes (#513) `@ElamC`
    * `Document`, `Index` and `Task` are now class and the attributes of these classes are now accessible via dot notation:
    * `document.offset`, `document.limit`, `document.total`
    * `task.uid`, `task.duration`, `task.status` ...
* Add pipenv install in the CONTRIBUTING guide (#519) `@brunoocasali`
* Bump mako from 1.2.1 to 1.2.2 due to security issue (#528) 

Thanks again to `@ElamC,` `@alallema,` `@brunoocasali!` 🎉


Co-authored-by: alallema <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change The related changes are breaking for the users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Class for every ressources
2 participants