Skip to content

Build docs with sphinx-gallery#195

Merged
lasofivec merged 7 commits intoToFuProject:develfrom
flothesof:sphinx_gallery
Sep 13, 2019
Merged

Build docs with sphinx-gallery#195
lasofivec merged 7 commits intoToFuProject:develfrom
flothesof:sphinx_gallery

Conversation

@flothesof
Copy link
Copy Markdown
Contributor

Hi all,

this is a first draft of sphinx-gallery generation of examples.
Let me know what you think of it.

Regards,
Florian

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Sep 13, 2019

Hello @flothesof! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 318:80: E501 line too long (84 > 79 characters)

Line 5:80: E501 line too long (87 > 79 characters)
Line 10:80: E501 line too long (111 > 79 characters)
Line 17:80: E501 line too long (84 > 79 characters)
Line 18:80: E501 line too long (83 > 79 characters)
Line 28:80: E501 line too long (85 > 79 characters)
Line 30:1: E402 module level import not at top of file
Line 34:80: E501 line too long (82 > 79 characters)
Line 41:80: E501 line too long (114 > 79 characters)
Line 47:80: E501 line too long (103 > 79 characters)
Line 48:80: E501 line too long (84 > 79 characters)
Line 49:1: W391 blank line at end of file

Line 381:80: E501 line too long (89 > 79 characters)

Line 16:1: E402 module level import not at top of file

Comment last updated at 2019-09-13 14:40:05 UTC

@codecov-io
Copy link
Copy Markdown

codecov-io commented Sep 13, 2019

Codecov Report

Merging #195 into devel will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #195      +/-   ##
==========================================
+ Coverage   45.11%   45.11%   +<.01%     
==========================================
  Files          70       70              
  Lines       20639    20640       +1     
==========================================
+ Hits         9311     9312       +1     
  Misses      11328    11328
Impacted Files Coverage Δ
tofu/geom/utils.py 43.85% <100%> (+0.13%) ⬆️
tofu/geom/_core.py 64.05% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8a08c4...54ae89f. Read the comment docs.

@flothesof
Copy link
Copy Markdown
Contributor Author

I managed to get it to work, despite the windows specific problems. Among the fixes are things like this:

-            lsnvert = np.asarray(lsnvert, dtype=np.int64)
+            lsnvert = np.asarray(lsnvert, dtype=np.long)

There are probably some more of these to be found: they cause problems on Windows!

@Didou09 Didou09 added this to the New documentation milestone Sep 13, 2019
User specific ignore was already taken out some time ago... Somehow it sneak it way back in
Comment thread tofu/geom/_core.py
num_tot_structs += len(ss.Lim)

lsnvert = np.asarray(lsnvert, dtype=np.int64)
lsnvert = np.asarray(lsnvert, dtype=np.long)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure how this affects linux users, waiting for feedback from Laura

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm gonna test it in my machine, could you try it also in CentOS ?

Comment thread tofu/geom/utils.py
changes for PEP8 speaks
Copy link
Copy Markdown
Member

@Didou09 Didou09 left a comment

Choose a reason for hiding this comment

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

Ok, but:

  • waiting for feedback from Laura as other linux user
  • IMPORTANT : we do pull requests only to devel, which our common base working branch, PR to master are only done from devel, when we decide devel is advanced / stable enough to issue a release
    => I'm switching target of PR to devel

Thanks !

@Didou09 Didou09 changed the base branch from master to devel September 13, 2019 14:34
@Didou09 Didou09 self-requested a review September 13, 2019 14:35
@lasofivec
Copy link
Copy Markdown
Collaborator

Made changes for pep8, merged with devel, and tried on my Mac and Ubuntu. Should we wait for @Didou09 to try on CentOS?

Copy link
Copy Markdown
Collaborator

@lasofivec lasofivec left a comment

Choose a reason for hiding this comment

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

Great job, working on my configs

@Didou09 Didou09 self-requested a review September 13, 2019 15:02
@lasofivec
Copy link
Copy Markdown
Collaborator

Forgot to mention: the example is 🔥

@lasofivec lasofivec merged commit 37e6db5 into ToFuProject:devel Sep 13, 2019
@Didou09 Didou09 mentioned this pull request Nov 20, 2019
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.

5 participants