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

Update readthedocs page #466

Merged
merged 9 commits into from
Feb 22, 2021
Merged

Update readthedocs page #466

merged 9 commits into from
Feb 22, 2021

Conversation

fehiepsi
Copy link
Member

No description provided.

@fehiepsi fehiepsi added the WIP label Feb 20, 2021
"numpyro>=0.2.4",
"jax>=0.1.57",
"jaxlib>=0.1.37",
],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fritzo Do you have a config file for black? I run make format but it causes a lot of changes.

Copy link
Member

@fritzo fritzo Feb 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our only config file is setup.cfg but we do not configure black. Maybe you need to update isort, whose black support was added only in 5.0?

pip install -U isort black flake8

Copy link
Member Author

@fehiepsi fehiepsi Feb 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just updated the packages. I am not sure what is the default behavior but it seems to me the changes here look more consistent with what I think black will do. For example,

        params = {
            "eps_g": {},
            "eps_i": {},
        }

should be in 1 line

params = {"eps_g": {}, "eps_i": {}}

if it fits into an 88-length (black default line length) line. Anyway, I will revert the changes to not populate this PR, whose current changes might be caused by some mysterious config files in my system.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm that's weird. I especially don't understand why linting (black --check .) passed both before and after your black changes e7dc9d1 Does make lint pass on your system?

Copy link
Member Author

@fehiepsi fehiepsi Feb 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you are right. black --check . passed now, and make format does not change files anymore. It is weird... I guess black supports two formatting rules for parentheses and based on "cache" or something like that, black will behave differently... Anyway, I will revert unrelated changes because make format seems to work now in my system.

a side note: the current behavior is a bit different from what I understand from How black wraps lines.

update2: yeah, there seems to have two acceptable behaviors for black (also the related issue on how black deals with trailing commas on list/tuple/dict/...). Hopefully, there will be a flag in the next black release, that makes the behavior consistent.

@fehiepsi
Copy link
Member Author

@fritzo There are a few black changes remaining, hope that they are fine. I have incorporated (and tested) the following changes that I made to rtd page:

  • add the ability to add examples/tutorials to index.rst file
  • add the ability to generate thumbnail galleries (this is not necessary)
  • add colab links to rendered tutorials
  • each tutorial/example has a link that points to the source file (with correct version) in github.

I need to add __version__ to funsor.__init__.py to generate correct links for local versions (not sure if it is needed for funsor.pyro.ai). We expose __version__ in pyro and numpyro so it is fine to do it here I guess.

@fritzo
Copy link
Member

fritzo commented Feb 22, 2021

Thanks @fehiepsi, this is great!

@fritzo fritzo merged commit 0f81eb0 into master Feb 22, 2021
@fritzo fritzo deleted the doc-revise branch February 22, 2021 17:51
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.

2 participants