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

Clarify documentation of iarange #545

Merged
merged 1 commit into from
Nov 9, 2017
Merged

Clarify documentation of iarange #545

merged 1 commit into from
Nov 9, 2017

Conversation

fritzo
Copy link
Member

@fritzo fritzo commented Nov 9, 2017

Fixes #542

This aims to clarify the purpose of iarange. The docs are getting a little long, so I may trim them back down when we have a dedicated tutorial. I haven't addressed context managers since that's a basic python concept outside of Pyro.

@fritzo
Copy link
Member Author

fritzo commented Nov 9, 2017

@srush Thanks for pointing this out. Do these changes help?

@srush
Copy link

srush commented Nov 9, 2017

Yup, very clear!

Although of course a named version without indexes like:

with latent.data_group.exchangeable(data) as group, batch:
    for b in batch:
        group.x.add().observe(b)

Seems more safe to me...

@fritzo
Copy link
Member Author

fritzo commented Nov 9, 2017

Ready to merge (tests timed out).

@eb8680 eb8680 merged commit c36ac73 into dev Nov 9, 2017
@eb8680 eb8680 deleted the document-iarange branch November 9, 2017 23:52
@jpchen
Copy link
Member

jpchen commented Nov 10, 2017

theres one snag with deploying this which is that the docs track master, which means that other docs changes eg splitting LambdaPoutine into ScalePoutine + IndepPoutine will also be deployed and will be inconsistent with the release code. i suggest we wait until the next minor version/patch to deploy

@fritzo
Copy link
Member Author

fritzo commented Nov 10, 2017

Good point. Now is probably a good time to release a patch that includes the fix #533.

@fritzo
Copy link
Member Author

fritzo commented Nov 10, 2017

@neerajprad let's address this test timeout issue we're facing before we release a patch, so I won't have to click RESTART RESTART RESTART when we push to master 😄

@ngoodman
Copy link
Collaborator

and note that @null-a is updating the AIR tutorial in light of #533 (which makes AIR work better). it'd be great to have that deployed sooner than later...

@fritzo
Copy link
Member Author

fritzo commented Nov 10, 2017

Ok, I suggest we cut a patch release this morning. Then @null-a can later update AIR in dev and http://pyro.ai . Since the tutorials are not included the pip package, it should be ok to release a patch first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants