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

Blog post on using coordinates to model loops #133

Merged
merged 7 commits into from
Jul 23, 2018
Merged

Conversation

wtbarnes
Copy link
Member

This blog post is a tutorial for using the sunpy.coordinates module for modeling loops on the Sun. Any feedback (on the code and/or text) is greatly appreciated!

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

The only comment with this is that some of the code in the cells is too wide for the width of the code cells. We should probably try and make the code cells wider?

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

Also is there a "download this notebook" link we can build in somewhere?

@wtbarnes
Copy link
Member Author

Yeah I agree that the code cells are annoyingly narrow. I tried to avoid long lines when writing the notebook so that it is more readable. Glancing at nbsphinx and even nbconvert, it doesn't seem there's an easy way to adjust the code cell width.

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

I would take a guess that it's being limited by our theme css. @nabobalis ?

@wtbarnes
Copy link
Member Author

Re: a download link, I have a PR ready for #132 that will automatically generate a link to the static notebook and the binder notebook at the top of each post. Because I'm bad at branching, it is easier to merge this PR first and then I'll submit the other.

@wtbarnes
Copy link
Member Author

Oh true. That box on the left is pretty wide. Maybe making that a bit more narrow would help.

@nabobalis
Copy link
Contributor

Maybe? We can change the CSS tho.

Copy link
Contributor

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

We could also CSS out the input and output cell markers? That gives us quite a bit more space?

@nabobalis
Copy link
Contributor

Probably? I don't think we touch those normally in the CSS.

@nabobalis
Copy link
Contributor

Is the problem the code cells have scroll bars?

@wtbarnes
Copy link
Member Author

Could do. That may involve a PR to nbsphinx

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

Also @wtbarnes I would put your bio at the top.

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

That may involve a PR to nbsphinx

We could just do it in the sunpy theme I would think.

@wtbarnes
Copy link
Member Author

Feels like it might be too distracting at the top

@wtbarnes
Copy link
Member Author

To strip out the In[],Out[] tags, I think you might have intervene at the nbconvert step which is controlled by nbsphinx... I may be wrong here.

@nabobalis
Copy link
Contributor

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

I think sunpy/sunpy-sphinx-theme#58 should do the job

@wtbarnes
Copy link
Member Author

Sure, but I think the issue is getting nbsphinx to use your custom template (i.e. tpl file). See spatialaudio/nbsphinx#132. Currently, doesn't look like the nbsphinx supports custom templates.

@wtbarnes
Copy link
Member Author

image

@wtbarnes
Copy link
Member Author

@Cadair nice

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

How much control does nbsphinx give you over the colour scheme of the cells? I don't like the lack of contrast in the current ones, not great for accessibility etc.

@wtbarnes
Copy link
Member Author

I think it just depends on the syntax highlighter you're using.

@wtbarnes
Copy link
Member Author

You can set the pygments style in the theme.conf file of the theme

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

Nabil you wanna roll out a theme update before or after merging this?

@nabobalis
Copy link
Contributor

Err, either?

@Cadair
Copy link
Member

Cadair commented Jul 23, 2018

I leave it up to you.

@nabobalis
Copy link
Contributor

Done. You can revert it back to normal now.

Copy link
Contributor

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

Once more

@Cadair Cadair merged commit b869014 into sunpy:master Jul 23, 2018
@wtbarnes
Copy link
Member Author

🎉

wtbarnes added a commit to wtbarnes/sunpy.org that referenced this pull request Jul 23, 2018
This reverts commit b869014, reversing
changes made to fd05b8e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants