Skip to content

Implementation of Deque using array #174

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

Implementation of Deque using array #174

wants to merge 8 commits into from

Conversation

Jahnavi-Jonnalagadda
Copy link

Added code for Implementation of Deque using array

References to other Issues or PRs or Relevant literature

Fixes #120

Brief description of what is fixed or changed

Implemented Deque using arrays by adding popright() and appendleft() functions to the existing code.

Other comments

Code raises exception if the queue is not doubly ended

@Jahnavi-Jonnalagadda
Implementation of Deque using array
2b4738c 
Added code for Implementation of Deque using array
czgdp1807
czgdp1807 reviewed Mar 19, 2020
View reviewed changes
czgdp1807
czgdp1807 reviewed Mar 19, 2020
View reviewed changes
czgdp1807
czgdp1807 reviewed Mar 19, 2020
View reviewed changes
czgdp1807
czgdp1807 reviewed Mar 19, 2020
View reviewed changes
czgdp1807
czgdp1807 reviewed Mar 19, 2020
View reviewed changes
@codecov
Copy link

codecov bot commented Mar 19, 2020 •
edited
Loading

Codecov Report

Merging #174 into master will increase coverage by 0.993%.
The diff coverage is 38.709%.

@@              Coverage Diff              @@
##            master      #174       +/-   ##
=============================================
+ Coverage   96.735%   97.729%   +0.993%     
=============================================
  Files           23        24        +1     
  Lines         1746      2246      +500     
=============================================
+ Hits          1689      2195      +506     
+ Misses          57        51        -6
Impacted Files Coverage Δ
...datastructs/miscellaneous_data_structures/queue.py 89.175% <38.709%> (-10.825%) ⬇️
pydatastructs/trees/heaps.py 97.802% <0.000%> (-0.532%) ⬇️
...datastructs/linear_data_structures/linked_lists.py 99.765% <0.000%> (-0.235%) ⬇️
pydatastructs/graphs/__init__.py 100.000% <0.000%> (ø)
pydatastructs/graphs/algorithms.py 100.000% <0.000%> (ø)
pydatastructs/linear_data_structures/arrays.py 100.000% <0.000%> (ø)
pydatastructs/linear_data_structures/__init__.py 100.000% <0.000%> (ø)
...astructs/miscellaneous_data_structures/__init__.py 100.000% <0.000%> (ø)
pydatastructs/linear_data_structures/algorithms.py 100.000% <0.000%> (ø)
... and 3 more

Impacted file tree graph

@czgdp1807
Copy link
Member

Nice work. I have made some suggestions to describe my point.

@czgdp1807
Copy link
Member

Are you participating through GSSoC, 2020? Or would you like to count this contribution as RGSoC, 2020?

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 19, 2020 via email

@czgdp1807
Copy link
Member

Please add some tests to increase the diff coverage.
P.S. If you are applying for RGSoC, 2020 in our project then please mention this PR in your application.

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 19, 2020 via email

@czgdp1807
Copy link
Member

@Jahnavi-Jonnalagadda Are you working on it? There are some things which are to be addressed before this gets merged.

@czgdp1807
Copy link
Member

Btw, what is diff coverage? Sorry for the basic question.

The lines marked in red here aren't executed even once by the tests i.e., those lines aren't tested so how can someone rely on them?

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 26, 2020 via email

@czgdp1807
Copy link
Member

Have you gone through #174 (comment) ?
If you still are not able to get it then please let me know.

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 26, 2020 via email

czgdp1807
czgdp1807 reviewed Mar 27, 2020
View reviewed changes
@czgdp1807
Copy link
Member

Am I supposed to change the code of those lines in red?

Tests are to be added for the lines in red in the following function,

pydatastructs/pydatastructs/miscellaneous_data_structures/tests/test_queue.py

Line 17 in 6cc969c

def test_ArrayQueue():

In addition, LinkedListQueue should also have the definitions of appendleft and popright methods.

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 27, 2020 via email

@czgdp1807
Copy link
Member

There red lines at https://codecov.io/gh/codezonediitj/pydatastructs/pull/174/diff
For analysing the coverage locally, see, https://github.com/codezonediitj/pydatastructs#testing

@Jahnavi-Jonnalagadda
Copy link
Author

Am I supposed to change the code of those lines in red?

Tests are to be added for the lines in red in the following function,

pydatastructs/pydatastructs/miscellaneous_data_structures/tests/test_queue.py

Line 17 in 6cc969c

def test_ArrayQueue():

In addition, LinkedListQueue should also have the definitions of appendleft and popright methods.

@Jahnavi-Jonnalagadda
Copy link
Author

Sorry for this, I've closed the PR by mistake. I actually thought of commenting.

@czgdp1807
Copy link
Member

Any updates? After two days, Please take over label will be attached so that someone else can continue it.

@Jahnavi-Jonnalagadda
Copy link
Author

Jahnavi-Jonnalagadda commented Mar 31, 2020 via email

@czgdp1807 czgdp1807 added Please take over PRs that can be continued by anyone. and removed gssoc20 labels Apr 2, 2020
@HarsheetKakar
Copy link
Contributor

Do you want Queue to be removed and implement Deque instead or you want a different class?

@czgdp1807
Copy link
Member

As can be seen in the patch, an extra keyword argument is required to be added to enable appendleft and popright operations on the Queue. So, just small additions are needed to class Queue with some tests.

@HarsheetKakar
Copy link
Contributor

As can be seen in the patch, an extra keyword argument is required to be added to enable appendleft and popright operations on the Queue. So, just small additions are needed to class Queue with some tests.

so just the tests need to be updated?

@czgdp1807
Copy link
Member

so just the tests need to be updated?

Yes.

@HarsheetKakar
Copy link
Contributor

so just the tests need to be updated?

Yes.

implementation is a bit off, need some time to correct it.

@HarsheetKakar
Copy link
Contributor

if we use circular array implementation of Deque we cant use DODA, since it will add none at the end of the array and we want space between rear and front, we would have to first add circular dynamic array first and then move to Deque. Although this comment suggests that collections.deque uses linked list instead of list, so if random access is not that important we can drop this idea.

@czgdp1807
Copy link
Member

Well, then ignore the arrays in case of doubly ended queue, just raise an error if array implementation is asked for when doubly_ended=True.

@HarsheetKakar
Copy link
Contributor

Well, then ignore the arrays in case of doubly ended queue, just raise an error if array implementation is asked for when doubly_ended=True.

let me try working on circular dynamic queue. If it works then we might just have better implementation than python itself.

@czgdp1807
Copy link
Member

Closing in favour of #247

@czgdp1807 czgdp1807 closed this Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@czgdp1807 czgdp1807 czgdp1807 left review comments

Assignees
No one assigned
Labels
miscellaneous_data_structures Please take over PRs that can be continued by anyone.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Array implementation of doubly ended queue (deque)
3 participants
@Jahnavi-Jonnalagadda @czgdp1807 @HarsheetKakar