Skip to content
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

Add tests for all of the Geometry types #26

Open
iAmChidiebereh opened this issue Apr 6, 2022 · 17 comments
Open

Add tests for all of the Geometry types #26

iAmChidiebereh opened this issue Apr 6, 2022 · 17 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@iAmChidiebereh
Copy link
Contributor

iAmChidiebereh commented Apr 6, 2022

Currently, only the MultiLineString, MultipiPoint and Random is being tested. Add validity checks to the rest of the GeoJson types and other places, where appropriate.

@iAmChidiebereh
Copy link
Contributor Author

Hi @patricoferris, I need your consent to work on this.
I await your response. Thank you :))

@IIITM-Jay
Copy link
Contributor

IIITM-Jay commented Apr 6, 2022

Hi @streetCoderr , actually I am working on #5, all these are come under that only. Like in a few days I am thinking to write some more test cases on others too.

@iAmChidiebereh
Copy link
Contributor Author

iAmChidiebereh commented Apr 6, 2022

Hi @IIITM-Jay, I don't understand what's going on.
Are you stacking issues up???
Because if you had an issue from last week that you are still working on, I don't understand why you had to pick up Shivangi's issue (even without @patricoferris consent) instead of focusing on yours.
She clearly stated that she has exams to write this week, but would be back next week for the issue. You should have left it for her then, or for anyone that has no issue they are currently working on.

I opened this issue because I saw that nobody was working on it. I thought you were done with the test you wanted to add. You did not indicate anything on going further.
Then, when I saw you picked up Shivangi's issue, I was finally convinced you were no longer working on it because a person can't be working on 2 issues at the same time. I'm surprised to hear from you now that you are still on it.

I don't even know what to say or do anymore.
It's alright.

@patricoferris
Copy link
Collaborator

Hi both,

Thank you for the amount of enthusiasm you are both showing to this project! But let's bare in mind that it is, afterall, just a small open-source library.

Because if you had an issue from last week that you are still working on, I don't understand why you had to pick up Shivangi's issue (even without @patricoferris consent) instead of focusing on yours.
She clearly stated that she has exams to write this week, but would be back next week for the issue. You should have left it for her then, or for anyone that has no issue they are currently working on.

I appreciate the considerate nature of your response, but I think @IIITM-Jay did the right thing which was to check if the issue was being worked on, I replied giving more help and some days to make sure no-one was and then went ahead and worked on the issue. I will create another issue for @SHIVANGISINGH1 to give them an opportunity to work on this project, that's no problem :))

@IIITM-Jay if you did intend to do some more work on the issue, please do say, I am trying to make sure everyone gets a fair chance to contribute.

The confusion is mainly my fault for having a very open ended issue with #5 (I'll fix this now). You both have unmerged PRs that I'm trying to review, so in the mean time let's focus on those. I'll turn this issue into a more specific one, and once either of you have your PR merged (or another person/Outreachy applicant comes along) feel free to ask to work on it then.

@patricoferris patricoferris changed the title Add more validity checks Add tests for all of the Geometry types Apr 6, 2022
@patricoferris
Copy link
Collaborator

This issue now focuses on adding tests for all of the Geometries (Point, MultiPoint etc.)

@patricoferris patricoferris added the good first issue Good for newcomers label Apr 6, 2022
@IIITM-Jay
Copy link
Contributor

Hi Sir, @patricoferris , Surely I will follow with whatever you said above. Thank you so much for guiding us regarding the same.

@IIITM-Jay if you did intend to do some more work on the issue, please do say, I am trying to make sure everyone gets a fair chance to contribute.

From now onwards, surely I will keep this in mind. Thank you 🙏

@iAmChidiebereh
Copy link
Contributor Author

Hi both,

Thank you for the amount of enthusiasm you are both showing to this project! But let's bare in mind that it is, afterall, just a small open-source library.

Because if you had an issue from last week that you are still working on, I don't understand why you had to pick up Shivangi's issue (even without @patricoferris consent) instead of focusing on yours.
She clearly stated that she has exams to write this week, but would be back next week for the issue. You should have left it for her then, or for anyone that has no issue they are currently working on.

I appreciate the considerate nature of your response, but I think @IIITM-Jay did the right thing which was to check if the issue was being worked on, I replied giving more help and some days to make sure no-one was and then went ahead and worked on the issue. I will create another issue for @SHIVANGISINGH1 to give them an opportunity to work on this project, that's no problem :))

@IIITM-Jay if you did intend to do some more work on the issue, please do say, I am trying to make sure everyone gets a fair chance to contribute.

The confusion is mainly my fault for having a very open ended issue with #5 (I'll fix this now). You both have unmerged PRs that I'm trying to review, so in the mean time let's focus on those. I'll turn this issue into a more specific one, and once either of you have your PR merged (or another person/Outreachy applicant comes along) feel free to ask to work on it then.

Alright Sir. There is no problem.
Thank you.

@Techbae22
Copy link
Contributor

Hi Sir @patricoferris,

Please can you assign this issue to me? This seems to be the only issue left and I would like to take it on. Do I have you consent to proceed?

Thank you.

@IIITM-Jay
Copy link
Contributor

IIITM-Jay commented Apr 12, 2022

Hi @Techbae22 , hope you are doing well...
There is no doubt that the project mentor @patricoferris Sir, will surely help and guide you through this, but if you got stuck at any moment please feel free to ask and let me know if I can also help. I have already written a test case on one of the geometries in the PR #14 , so if you wish you can take reference from that. Having said that, there is no rush at all and you can take your time :)) 😊

Please pardon me 🙏 if I have said something that I shouldn't be...

@Techbae22
Copy link
Contributor

Hi @Techbae22 , hope you are doing well... There is no doubt that the project mentor @patricoferris Sir, will surely help and guide you through this, but if you got stuck at any moment please feel free to ask and let me know if I can also help. I have already written a test case on one of the geometries in the PR #14 , so if you wish you can take reference from that. Having said that, there is no rush at all and you can take your time :)) 😊

Please pardon me 🙏 if I have said something that I shouldn't be...

Hello @IIITM-Jay,
This is so kind of you. I appreciate your offer to help and it has been noted. If I get stuck anywhere, I would do well to reach out to you. Thank you

@Techbae22
Copy link
Contributor

Hi @Techbae22 and @patricoferris Sir, If both of you don't mind, may I take two geometries and on the rest two @Techbae22 will take it up... as only four of the geometries are remaining to be tested

Hello @IIITM-Jay,

I have been studying the code and I feel I understand it well enough to take on all the test cases as they follow similar pattern. However, I appreciate your concern but please this issue has been assigned to me. Until I indicate or reach out to @patricoferris that I have any issues, I would appreciate it if you can hold on.

Thank you.

@IIITM-Jay
Copy link
Contributor

IIITM-Jay commented Apr 13, 2022

ok no problems then you can carry on... I thoght you were not working that's why...

Thank you

@IIITM-Jay
Copy link
Contributor

@Techbae22 now yeah I am able to see that you have been forked the repo now .. great going 👍

@patricoferris
Copy link
Collaborator

@IIITM-Jay thank you for your continued enthusiasm, I was really happy to see supportive comments like this one! However, please do not send comments like this one, especially since @Techbae22 replied less that 24 hours before you sent that so they were clearly still active on this issue. There have been many times I've cloned an upstream repository, worked on an issue and only at the end forked my own copy and changed the remotes to submit a PR.

Looking at someone's Github profile and making assumptions about what they're working on is not helpful nor in the spirit of Outreachy. If I'm concerned someone may have forgotten about an issue or might not have the time to work on it, then I will follow up on that.

I recognise this comes from a place of wanting to contribute and help, there is absolutely nothing wrong with opening more issues yourself to work on, the library has lots of aspects that can be improved. Moreover, getting a PR merged is only one small aspect of contributing to open-source.

@Techbae22 please do take as much time as you need and if you have any questions feel free to reach out on this issue or directly via email/discord too.

@Techbae22
Copy link
Contributor

@IIITM-Jay thank you for your continued enthusiasm, I was really happy to see supportive comments like this one! However, please do not send comments like this one, especially since @Techbae22 replied less that 24 hours before you sent that so they were clearly still active on this issue. There have been many times I've cloned an upstream repository, worked on an issue and only at the end forked my own copy and changed the remotes to submit a PR.

Looking at someone's Github profile and making assumptions about what they're working on is not helpful nor in the spirit of Outreachy. If I'm concerned someone may have forgotten about an issue or might not have the time to work on it, then I will follow up on that.

I recognise this comes from a place of wanting to contribute and help, there is absolutely nothing wrong with opening more issues yourself to work on, the library has lots of aspects that can be improved. Moreover, getting a PR merged is only one small aspect of contributing to open-source.

@Techbae22 please do take as much time as you need and if you have any questions feel free to reach out on this issue or directly via email/discord too.

Thank you @patricoferris. I will reach out if I have any questions

@Techbae22
Copy link
Contributor

@patricoferris,
Hi Sir, please the JSON values I'm supposed to use for the test cases, is there a particular place I'm supposed to get them from or can I use any desired JSON value?
Thank you.

@Techbae22
Copy link
Contributor

Techbae22 commented Apr 19, 2022

Hello Sir @patricoferris ,
I'm a kinda stuck on writing tests for Polygon.
Is there any Array function I can use to combine the interior and exterior rings?
I have already written tests for point and linestring.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants