Skip to content

[Fix] Created dataset module and other module changes.#11

Merged
pieterprovoost merged 5 commits into
iobis:masterfrom
ayushanand18:gsoc2022-dev-aa
Apr 27, 2022
Merged

[Fix] Created dataset module and other module changes.#11
pieterprovoost merged 5 commits into
iobis:masterfrom
ayushanand18:gsoc2022-dev-aa

Conversation

@ayushanand18
Copy link
Copy Markdown
Collaborator

Overview of changes

  • I have changed the pyobis/nodes, occurrences, checklist, and taxa modules to integrate them with OBIS v3 API and updated README with new example usage for the modules.
  • I have also created a new dataset module to connect to the obis/dataset endpoint.
  • I have also deleted pyobis/groups and pyobis/resources because they have become obsolete.
    (Refer: [Fix] Integrated existing pyobis modules with OBIS v3 API #8 (comment) )

Notes for reviewers

You can run tests on your local machine whether the above 5 modules are working as expected.

Questions for reviewers

No questions for reviewers.

Desc. I have successfully updated the checklist and dataset module to match with the new API. Permanentally deleted resources module.
I have updated the Nodes module and also updated the proposed changes.
Desc. I switched to a new branch to keep clean code, ready for a PR.
Pulled a commit from iobis/pyobis master which changed links to docs and source repo
I have completed writing dataset module as well as did some minor changes in functions of taxa module. I have also updated the README with the new terminology, and updated module usage.
@ayushanand18
Copy link
Copy Markdown
Collaborator Author

@pieterprovoost @ocefpaf I'm sorry for pinging you here, but just wanted to request feedback on this PR.

I know you'd have been very busy since a couple of weeks due to the GSoC timeline. May I ask for the code review, I have removed the resources module plus also included a new module for dataset endpoint. I have also updated the README with the new syntax and nomenclature.

Additionally, all tests in this branch have failed but I have worked on them and we will need one more patch to fix them (you can see the workflow run on my fork, https://github.com/ayushanand18/pyobis/actions/runs/2184556665 Status: Tests ).

Your feedback will help me improve this PR. Thanks in advance! :)

@pieterprovoost pieterprovoost merged commit 87ba216 into iobis:master Apr 27, 2022
@ocefpaf
Copy link
Copy Markdown
Collaborator

ocefpaf commented Apr 27, 2022

@ayushanand18 most of the GSoC mentors are in the IOOS Code Sprint this week and we are quite busy.
Also, I already touched base with you on this but if you can send smaller PRs with a single context it would help us a lot to review and merge. This PR removed a lot of files, changed config stuff, and added tests. Those should all be in separated PRs.

@ayushanand18
Copy link
Copy Markdown
Collaborator Author

Thanks a ton @ocefpaf for the review. I am sorry for adding so many changes to a single PR.
I'll focus on smaller and more easy to review PRs in future. Thanks again for your review and once again very sorry to disturb amidst a tight schedule.

Thank you so much @pieterprovoost :)

@ocefpaf
Copy link
Copy Markdown
Collaborator

ocefpaf commented Apr 28, 2022

I am sorry for adding so many changes to a single PR.

Don't be. There is nothing wrong with what you did. This is about how communities/people work and that usually takes time and iteration to find the best spot. Just be patient with us and we'll fine tune the PRs to take the most of your contributions.

@ayushanand18
Copy link
Copy Markdown
Collaborator Author

Thank you so much @ocefpaf :) I have learned so much from the Open Source community and especially during my time spent understanding the pyobis project. I have so much to learn more about, and I am excited about the future!

@ayushanand18 ayushanand18 deleted the gsoc2022-dev-aa branch May 21, 2022 09:52
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