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

STY: Higher contrast [circle front] #5230

Merged
merged 12 commits into from Jun 1, 2018

Conversation

Projects
None yet
6 participants
@larsoner
Member

larsoner commented May 25, 2018

No description provided.

@larsoner

This comment has been minimized.

Member

larsoner commented May 25, 2018

@mathurinm @jasmainak @dengemann feel free to decide if you like this higher-contrast one better (tango):

https://8109-1301584-gh.circle-artifacts.com/0/html/auto_tutorials/plot_mne_dspm_source_localization.html

And feel free to try other styles quickly if you want:

http://pygments.org/demo/6745072/?style=tango

@codecov

This comment has been minimized.

codecov bot commented May 25, 2018

Codecov Report

Merging #5230 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5230      +/-   ##
==========================================
+ Coverage   88.13%   88.17%   +0.04%     
==========================================
  Files         358      358              
  Lines       65784    66053     +269     
  Branches    11191    11356     +165     
==========================================
+ Hits        57976    58242     +266     
+ Misses       4983     4980       -3     
- Partials     2825     2831       +6
@agramfort

This comment has been minimized.

Member

agramfort commented May 26, 2018

LGTM

@jasmainak

This comment has been minimized.

Member

jasmainak commented May 26, 2018

I'm not a color expert but I like 'default' the best.

@larsoner

This comment has been minimized.

@agramfort

This comment has been minimized.

Member

agramfort commented May 27, 2018

@jasmainak

This comment has been minimized.

Member

jasmainak commented May 27, 2018

hmm ... but I don't understand. On the pygments website, default had black fonts but on the link Eric shared, it is gray ... that's why the contrast is low. Perhaps something else is overriding this color combination?

larsoner added some commits May 28, 2018

@larsoner

This comment has been minimized.

Member

larsoner commented May 28, 2018

There was a tricky bug where bootstrap was overriding the highlight CSS. The latest commit should fix it

@larsoner

This comment has been minimized.

Member

larsoner commented May 28, 2018

(workaround found in rtfd/sphinx_rtd_theme#166)

@jasmainak

This comment has been minimized.

Member

jasmainak commented May 28, 2018

oh wow! That sounds like a non-trivial fix

@agramfort

This comment has been minimized.

Member

agramfort commented May 28, 2018

@dengemann

This comment has been minimized.

Member

dengemann commented May 28, 2018

I find acceptable the brighter high-contrast / light background schemes:

screenshot 2018-05-28 22 09 20

screenshot 2018-05-28 22 09 12

screenshot 2018-05-28 22 09 00

screenshot 2018-05-28 22 08 47

screenshot 2018-05-28 22 08 22

screenshot 2018-05-28 22 08 05

@larsoner

This comment has been minimized.

Member

larsoner commented May 28, 2018

How can you tell when @agramfort is drowning in emails? When CircleCI artifacts being more than one click away is insurmountable :)

This is default:

https://8114-1301584-gh.circle-artifacts.com/0/html/auto_tutorials/plot_mne_dspm_source_localization.html

@dengemann

This comment has been minimized.

Member

dengemann commented May 28, 2018

@larsoner can we try those above with a light background?

@jasmainak

This comment has been minimized.

Member

jasmainak commented May 28, 2018

LGTM after bugfix. Other styles should also look better I guess.

@larsoner

This comment has been minimized.

Member

larsoner commented May 28, 2018

@dengemann can you be more specific as to what you mean? You posted 6 options are you asking if we can tweak the background color of each of them? Yes you can tweak CSS in chrome aas much as you want and post screenshots.

If you want to get circle to render all 6 feel free to push commits to my branch, waiting after each until the build is complete

@dengemann

This comment has been minimized.

Member

dengemann commented May 28, 2018

@mathurinm

This comment has been minimized.

Contributor

mathurinm commented May 28, 2018

I'll chip in. I like the ones which

  • do no put some values in bold
  • have a white background
  • do not highlight stuff
  • do not write comment in light gray
  • do not use flashy colors

Borland lgtm

larsoner added some commits May 28, 2018

@larsoner

This comment has been minimized.

Member

larsoner commented May 28, 2018

To keep the number of permutations and iterations reasonable, can people you make a PR to my branch with your favorite option? I'll merge each and let it build stop we can all see (or post ascreenshot). I fear each person giving half a dozen options or constraints is too imprecise for aesthetic convergence.

For example, for now links are distinguished by being bold but you can change this if you want (look at the diff and you will see it). So it's not as simple as just setting it to borland and letting it build :(

@larsoner

This comment has been minimized.

@dengemann

This comment has been minimized.

Member

dengemann commented May 29, 2018

@agramfort

This comment has been minimized.

Member

agramfort commented May 29, 2018

@dengemann

This comment has been minimized.

Member

dengemann commented May 29, 2018

@massich

This comment has been minimized.

Contributor

massich commented May 30, 2018

My 2 cents,

I like when the code, the output and the page background have slightly different colors. It really helps me with my ADD.

I like this background selection:
https://8125-1301584-gh.circle-artifacts.com/0/html/auto_tutorials/plot_mne_dspm_source_localization.html

this takes me more time to skim:
https://8130-1301584-gh.circle-artifacts.com/0/html/auto_tutorials/plot_mne_dspm_source_localization.html

Then regarding the fonts between those 2, maybe I like the one where the strings are red.

@agramfort

This comment has been minimized.

Member

agramfort commented May 30, 2018

@massich

This comment has been minimized.

Contributor

massich commented May 30, 2018

larsoner added some commits May 30, 2018

@larsoner

This comment has been minimized.

Member

larsoner commented May 30, 2018

I have no idea why some examples are effectively running with WARNING log level -- some examples are running with the INFO and others WARNING :(

In any case, rendered version with different colors for code/output (now using SG default for output actually) can be seen here, where I used verbose=True to force it to print:

https://8160-1301584-gh.circle-artifacts.com/0/html/auto_tutorials/plot_mne_dspm_source_localization.html

@agramfort

This comment has been minimized.

Member

agramfort commented May 31, 2018

@larsoner

This comment has been minimized.

Member

larsoner commented May 31, 2018

@massich

This comment has been minimized.

Contributor

massich commented May 31, 2018

@dengemann

This comment has been minimized.

Member

dengemann commented May 31, 2018

@agramfort agramfort merged commit 05a09f7 into mne-tools:master Jun 1, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/project 88.17% (+0.04%) compared to 005f4ab
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@larsoner larsoner deleted the larsoner:bikeshed branch Jun 1, 2018

britta-wstnr added a commit to britta-wstnr/mne-python that referenced this pull request Jul 5, 2018

STY: Higher contrast [circle front] (mne-tools#5230)
* STY: Higher contrast [circle front]

* STY: default [circle front]

* STY: remove custom

* FIX: Workaround [circle front]

* FIX: Fix [circle front]

* STY: abap [circle front]

* STY: pastie [circle front]

* STY: borland and non-bold [circle front]

* FIX: Fix styles [circle front]

* FIX: bgcolors [circle front]

* FIX: Dont set in file [circle front]

* FIX: Fix order [circle front]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment