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

Entire code base reformat with black. Added github workflow for ci. #379

Merged
merged 20 commits into from
Mar 25, 2021

Conversation

ejolly
Copy link
Collaborator

@ejolly ejolly commented Mar 22, 2021

Also added vscode settings and recommended extensions

@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #379 (b16a029) into master (da5f37d) will decrease coverage by 0.11%.
The diff coverage is 74.16%.

❗ Current head b16a029 differs from pull request most recent head 86534f3. Consider uploading reports for the commit 86534f3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #379      +/-   ##
==========================================
- Coverage   70.47%   70.36%   -0.12%     
==========================================
  Files          31       31              
  Lines        5528     5520       -8     
==========================================
- Hits         3896     3884      -12     
- Misses       1632     1636       +4     
Impacted Files Coverage Δ
nltools/data/brain_data.py 66.76% <ø> (ø)
nltools/datasets.py 0.00% <0.00%> (ø)
nltools/plotting.py 8.76% <0.00%> (ø)
nltools/stats.py 65.83% <ø> (-0.53%) ⬇️
nltools/tests/test_file_reader.py 100.00% <ø> (ø)
nltools/external/srm.py 75.86% <46.42%> (-0.24%) ⬇️
nltools/prefs.py 59.37% <47.36%> (ø)
nltools/utils.py 67.27% <47.36%> (ø)
nltools/analysis.py 51.77% <47.61%> (ø)
nltools/data/design_matrix.py 72.13% <53.48%> (-0.28%) ⬇️
... and 28 more

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 e2f78a0...86534f3. Read the comment docs.

@ejolly
Copy link
Collaborator Author

ejolly commented Mar 23, 2021

@ljchang This is ready to merge in. All tests are passing except for one which I think you might be able to fix. Any chance you can take a quick look: https://github.com/cosanlab/nltools/runs/2171188010?check_suite_focus=true#step:6:47

We're also having some weird platform issues around MAXINT on Windows, but fine on Ubuntu and macOS. As of now we don't support Python 3.9 as of yet because several of our dependencies are not Python 3.9 ready either. But 3.9 is still new enough that that's ok I think.

Also our sphinx auto-gallery all builds without errors now and I can hook it up to deploy somewhere. Did you still want to use readthedocs or should I switch it to github pages?

The total list of changes include:

  • switching all CI to github actions which does the following:
    • checks code formatting via black and will kill build if a commit or PR isn't formatted
    • runs pytest with parallelization
    • runs code-coverage and pushes to coveralls.io
    • in a parallel to all of the above, sets up a separate system to that installs the package and uses it to build the docs including the auto-gallery. Then it builds the HTML and can be pushed somewhere (e.g. github pages, readthedocs, netlify)
  • removing pretty much all traces of Python 2 code (e.g. from __future__ imports), six, etc
  • Fixes support Path objects #369
  • Fixes check that input to stats.fdr is between 0 and 1 #364
  • Starts on Major refactoring #377
  • Pins our max pandas version to < 1.2, because deepdish needs to support it first for all our .h5 support

@ljchang
Copy link
Member

ljchang commented Mar 23, 2021

I'll start looking through code, I would like to move documentation to github if possible, sick of debugging RTD.

Is sphinx-gallery building on github actions? if so, that's awesome!

@ejolly
Copy link
Collaborator Author

ejolly commented Mar 23, 2021

ok @ljchang I fixed that failing test by giving our simulator classes a random seed and just pinning it for testing purposes. Also docs work great on gh-pages! https://cosanlab.github.io/nltools/

Did you want to setup up a custom domain? Will probably merge this PR in shortly.

@ejolly ejolly merged commit def76a0 into master Mar 25, 2021
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

2 participants