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

Docs(Queue): Fix a PriorityQueue API missing bug #3927

Merged
merged 3 commits into from
Aug 24, 2016

Conversation

DjangoPeng
Copy link
Contributor

@DjangoPeng DjangoPeng commented Aug 19, 2016

As we all know there are 4 subclass of class QueueBase, but only 3 subclass(FIFOQueue, PaddingFIFOQueue, RandomShuffleQueue) descriptions in the API Docs here

So I add the description of class PriorityQueue() which are the comments in ../python/ops/data_flow_ops.py.
Start line 714 to 759 and you can check here

Add the description of  `class PriorityQueue()` API in /python/ops/data_flow_ops.py.
From line tensorflow#714 to tensorflow#759 you can check [here](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/data_flow_ops.py)
@teamdandelion
Copy link
Contributor

Thanks for the pull request! Unless I'm much mistaken, these files should be autogenerated from the source in data_flow_ops. @josh11b can you comment on what the right fix is?

@tensorflow-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

@teamdandelion teamdandelion added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Aug 19, 2016
@josh11b
Copy link
Contributor

josh11b commented Aug 19, 2016

The source for the documentation of tf.PriorityQueue lives in https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/data_flow_ops.py#L715

tensorflow/g3doc/api_docs/python/io_ops.md is automatically generated. Any changes to it will be overwritten the next time the documentation generator runs. In fact, its very first line is:

The fact that PriorityQueue is not being generated is an issue, which we can hopefully fix by adding PriorityQueue to the list here:
https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/io_ops.py#L82
(And hopefully then the doc generator will generate docs from the data_flow_ops.py file -- see https://www.tensorflow.org/versions/r0.10/how_tos/documentation/index.html for instructions on how to run the doc generator yourself. TL/DR: tools/docs/gen_docs.sh)

If, once the documentation is being generated, there are still improvements to be made, those changes should be made to data_flow_ops.py.

@josh11b josh11b added stat:awaiting response Status - Awaiting response from author and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Aug 19, 2016
@DjangoPeng
Copy link
Contributor Author

@josh11b So it is a missing mistake of the list, and when could you run the doc generator script?
And do you mean I don't need to push this commits this time?

@rmlarsen
Copy link
Member

@DjangoPeng @josh11b I think Josh's point was that if you add the correct entry to

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/io_ops.py#L82

then the documentation will get automatically generated (visibility pending our push). Please go ahead and make that change as well as any documentation updates to data_flow_ops.py in this PR.

@DjangoPeng
Copy link
Contributor Author

DjangoPeng commented Aug 23, 2016

@rmlarsen I have added PriorityQueue into the list, plz check

Add the class PriorityQueue to the list and it will automatically generate the corresponding documentation with the new description of PriorityQueue
@rmlarsen
Copy link
Member

@DjangoPeng Can you please revert the changes from io_ops.md, since this is an autogenerated file, and instead update the documentation in

https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/ops/data_flow_ops.py#L715

if needed.

@DjangoPeng
Copy link
Contributor Author

@rmlarsen I have reverted the change from io_ops.md, and it's unnecessary to update the data__flow_ops.py this time. Plz check.

@rmlarsen
Copy link
Member

@tensorflow-jenkins test this please

@rmlarsen rmlarsen merged commit 5423dc2 into tensorflow:master Aug 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes stat:awaiting response Status - Awaiting response from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants