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

[WIP] Nicer more structured docs #272

Merged
merged 30 commits into from Jul 24, 2022
Merged

Conversation

robintibor
Copy link
Contributor

@robintibor robintibor commented Jun 23, 2021

Aim:
Have more structure in the documentation so that people can more easily find all relevant documentation without going through the example list.

Maybe also try whatever can be done to make the documentation more beautiful.

For now added some "Quickstart" example, will do more in coming days to improve.

Copy link
Collaborator

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

some thoughts...

docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated

# Create the model
model = ShallowFBCSPNet(
in_chans=train_set[0][0].shape[0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

calling this n_channels seems so much more natural to me...

docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated
model = ShallowFBCSPNet(
in_chans=train_set[0][0].shape[0],
n_classes=4,
input_window_samples=train_set[0][0].shape[1],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
input_window_samples=train_set[0][0].shape[1],
n_samples=train_set[0][0].shape[1],

or n_times ...

examples/plot_bcic_iv_2a_moabb_cropped.py Show resolved Hide resolved
@bruAristimunha
Copy link
Collaborator

Hello @robintibor,

I am wondering if I could perhaps take advantage of your pull-request to change the site's theme from braindecode to pydata_sphinx_theme, such as numpy and mne. Would that be acceptable to you?

Probably, this involves a lot of small changes to make the site look nicer, and I would need some help.

Example: https://bruaristimunha.github.io/braindecode.github.io/master/index.html

@robintibor
Copy link
Contributor Author

Yeah, feel free to go ahead!

@bruAristimunha
Copy link
Collaborator

many thanks @robintibor!

I created a PR in your folk.

bruAristimunha and others added 4 commits July 5, 2022 19:03
changing braindecode documentation theme
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
@codecov
Copy link

codecov bot commented Jul 5, 2022

Codecov Report

Merging #272 (9962143) into master (6129ffb) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #272      +/-   ##
==========================================
- Coverage   82.66%   82.63%   -0.03%     
==========================================
  Files          53       53              
  Lines        3738     3738              
==========================================
- Hits         3090     3089       -1     
- Misses        648      649       +1     

@bruAristimunha
Copy link
Collaborator

bruAristimunha commented Jul 5, 2022

For me there are still several details to review to fully integrate this new theme. At mne it looks like this involved a team, we can just copy if the license allows. What do you think @agramfort?

@bruAristimunha
Copy link
Collaborator

A logo for braindecode would be very cool. Does anyone have any artistic skills?

@agramfort
Copy link
Collaborator

here is the rendered doc:

https://output.circle-artifacts.com/output/job/4c2e2a78-48ab-4c3d-9ace-b1601596fc1a/artifacts/0/dev/index.html

it's going in the right direction but it needs a few more iterations
eg to adjust the header bar.

thx @bruAristimunha for taking a stab at this 🙏

@bruAristimunha
Copy link
Collaborator

Besides the details and adjustments in the div, I was wondering if the colours are a good start.

In the light mode, it is not bad. I don't appreciated it much in the dark mode, and I suggest turning off this option. I was thinking about this palette in the light mode:

image

I don't know if someone will engage in this task with me, but I will try to centralize any decision here.

@agramfort
Copy link
Collaborator

agramfort commented Jul 6, 2022 via email

@bruAristimunha
Copy link
Collaborator

A first draft version of a logo:

As picture:
image

Vectorized version:
logo_braindecode

It's something to start with. Anyone want to give some feedback? @robintibor, @gemeinl, @agramfort

@agramfort
Copy link
Collaborator

how about something like this

Screenshot 2022-07-08 at 12 06 41

@bruAristimunha
Copy link
Collaborator

I liked Alexandre!

@sliwy
Copy link
Collaborator

sliwy commented Jul 8, 2022

How about putting the NN on the right inside the brain to match the left hemisphere?

something like this but with the brain from your options and EEG between the hemispheres:
image

in case, graphic from here: https://www.pngegg.com/en/png-bbroj

@bruAristimunha
Copy link
Collaborator

I like this option also @sliwy. The only thing I would add is a brain sign somewhere, but it could be near the name.

Let's wait for some maintainers and contributors to manifest, and we close with a vote. @hubertjb, @cedricrommel, @sylvchev, any contribution to the logo?

I don't know the entire team involved in braindecode very well, so if you want to invite more people to encourage participation, please do.

@sliwy
Copy link
Collaborator

sliwy commented Jul 8, 2022

Primitive version but something like that:

image

and more compact one:

image

I would still work on the contrast of the network graph inside the brain, maybe it is too similar to the brain color, or we can remove the brain coloring from the right hemisphere.

@agramfort
Copy link
Collaborator

@bruAristimunha can you see why CIs are red? it prevents us from looking at the rendered doc. 🙏

@bruAristimunha
Copy link
Collaborator

A thousand apologies for the mess, @agramfort.

It looks like a test on augmentations is returning a RunTimeError, which is pretty weird because I didn't touch this area. While trying to find out the cause of the error, I will locally compile the rendered and put it on my fork.

@agramfort
Copy link
Collaborator

agramfort commented Jul 18, 2022 via email

@bruAristimunha
Copy link
Collaborator

I will rebase the nicer-doc branch with the current master branch, but the error does not seem related.

In the meantime, I think I've made a first iteration of the new format for the site. I put it in my fork since I can't pass the CI. Link bellow:

https://bruaristimunha.github.io/braindecode.github.io/master/index.html

Can you look @agramfort and @robintibor? I still have to correct the colours and aspect ratio of the images. So it would be nice to comment without being related to these two points.

@agramfort
Copy link
Collaborator

agramfort commented Jul 19, 2022 via email

@bruAristimunha
Copy link
Collaborator

bruAristimunha commented Jul 19, 2022 via email

@bruAristimunha
Copy link
Collaborator

@agramfort
Copy link
Collaborator

agramfort commented Jul 22, 2022 via email

@bruAristimunha
Copy link
Collaborator

@agramfort, can you merge?

To do this merge, we will have to change some things in the braindecode.github.io repository. I think there will be other issues with this documentation change, but I think we can fix it in other iterations.

This was referenced Jul 23, 2022
@agramfort
Copy link
Collaborator

@bruAristimunha can you just fix the footer issue with the date?

@bruAristimunha
Copy link
Collaborator

Sure @agramfort. Done!

@agramfort agramfort merged commit 337d4c8 into braindecode:master Jul 24, 2022
@agramfort
Copy link
Collaborator

Thx @bruAristimunha !

@bruAristimunha
Copy link
Collaborator

It's alive =) @robintibor and @agramfort https://braindecode.org/master/index.html#

It's going to be pretty cool when we reach version 0.7 =)

@robintibor
Copy link
Contributor Author

Wow cool!

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.

None yet

5 participants