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

Importing iospi into simspi and transfer.py into simspi #5

Merged
merged 24 commits into from
Oct 16, 2021

Conversation

geoffwoollard
Copy link
Contributor

Importing iospi into simspi geoffwoollard@71f58af

@geoffwoollard
Copy link
Contributor Author

Linting and testing is getting this far

Successfully built ioSPI
Installing collected packages: wcwidth, setuptools-scm, blessed, scipy, flyingcircus-numeric, flyingcircus, raster-geometry, ioSPI
Successfully installed blessed-1.18.1 flyingcircus-0.1.4.0 flyingcircus-numeric-0.1.1.2 ioSPI-0.0.1 raster-geometry-0.1.4.1 scipy-1.7.1 setuptools-scm-6.0.1 wcwidth-0.2.5

done
#
# To activate this environment, use
#
#     $ conda activate base
#
# To deactivate an active environment, use
#
#     $ conda deactivate

ERROR: File "setup.py" or "setup.cfg" not found. Directory cannot be installed in editable mode: /home/runner/work/simSPI/simSPI
Error: Process completed with exit code 1.

@geoffwoollard
Copy link
Contributor Author

FYI I put myself down as maintainer in setup.py

@geoffwoollard
Copy link
Contributor Author

There is some issue with flake thinking black needs to change things. I think it's a line length issue. But I thought the default we were going for was 79 characters. I used this for (black ... -l 79) and passed deepsource. But then things failed during linting

Run $CONDA/bin/flake8 simSPI tests
  $CONDA/bin/flake8 simSPI tests
  shell: /bin/bash -e {0}
  env:
    pythonLocation: /opt/hostedtoolcache/Python/3.7.11/x64
    LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.7.11/x64/lib
simSPI:0:1: E902 FileNotFoundError: [Errno 2] No such file or directory: 'simSPI'
tests/test_multislice.py:16:37: BLK100 Black would make changes.
Error: Process completed with exit code 1.

This is a known issue: houndci/hound#1769

@ninamiolane ?

…ing for because it is on the same level as tests. I checked with ioSPI and put src
… length in docstrings, while black ignores these
@geoffwoollard
Copy link
Contributor Author

geoffwoollard commented Aug 20, 2021

Yes, the issue is definitely black and flake8 consistency. The missing directory issue went away with geoffwoollard@3f02d7c


Run $CONDA/bin/flake8 src tests
src/multislice.py:23:17: BLK100 Black would make changes.
tests/test_multislice.py:16:37: BLK100 Black would make changes.
Error: Process completed with exit code 1.

I checked the files it references, and perhaps its ending on (, the indentation on the next line, or black in the linting allowing for longer lines that 79 (if so, perhaps there's a way to pass -l 79 to black).

https://github.com/geoffwoollard/simSPI/blob/multi_slice/src/multislice.py#L23
https://github.com/geoffwoollard/simSPI/blob/multi_slice/tests/test_multislice.py#L16

@geoffwoollard
Copy link
Contributor Author

@ninamiolane @fredericpoitevin test_sim_volumes.py is failing tests. It was there from before and I didn't write it, nor the sim_volumes.py it tests. What should I do with it? Move them to a sim_volumes branch?

@geoffwoollard
Copy link
Contributor Author

Ok I moved transfer into simSPI. It's passing all the tests. I'm using a place holder for importing ioSPI for now in the simSPI.yml a la https://stackoverflow.com/questions/20101834/pip-install-from-git-repo-branch.

But when the ioSPI is merged into master and I can get fourier from there (by making a one line change to the yml to refer to compSPI/ioSPI). I'll just undo geoffwoollard@9f981d0

@geoffwoollard geoffwoollard changed the title Importing iospi into simspi Importing iospi into simspi and transfer.py into simspi Aug 25, 2021
@fredericpoitevin
Copy link
Member

@geoffwoollard I'll look back here once your PR on ioSPI has been merged. Will be easier to test dependency to it.

@geoffwoollard
Copy link
Contributor Author

@geoffwoollard I'll look back here once your PR on ioSPI has been merged. Will be easier to test dependency to it.

Ok ioSPI has been merged

@geoffwoollard
Copy link
Contributor Author

@fredericpoitevin Have a look

@geoffwoollard geoffwoollard merged commit ba5b6ff into compSPI:master Oct 16, 2021
@geoffwoollard
Copy link
Contributor Author

This got merged because I branched off of this when doing this PR #6 "Do continuous integration and codecoverage as in reduceSPI / ioSPI"

Copy link
Member

@fredericpoitevin fredericpoitevin left a comment

Choose a reason for hiding this comment

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

@geoffwoollard sorry I missed that, looks great! Would be nice to show a use case!


setuptools.setup(
name="simSPI",
maintainer="Geoffrey Woollard",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could add to this list going forward

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.

2 participants