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

Linting #21

Merged
merged 6 commits into from
Aug 25, 2021
Merged

Linting #21

merged 6 commits into from
Aug 25, 2021

Conversation

TomNicholas
Copy link
Member

@TomNicholas TomNicholas commented Aug 25, 2021

@jhamman I think there's a small problem with the pre-commit tools interacting with one another - my cmd line output is given below but no files are changed for me to commit. I've already run pre-commit run --all-files a few times to get to this point (those changes are what is committed in this PR), but now this happens every time. I think this means that black and isort are undoing each others changes to datatree/datatree.py? If I delete isort from the pre-commit-config.yaml then black passes.

(py38-mamba) tom@pop-os:~/Documents/Work/Code/datatree$ pre-commit run --all-files
Trim Trailing Whitespace.................................................Passed
Fix End of Files.........................................................Passed
Check Yaml...............................................................Passed
isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /home/tom/Documents/Work/Code/datatree/datatree/datatree.py

black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted datatree/datatree.py
All done! ✨ 🍰 ✨
1 file reformatted, 8 files left unchanged.

blackdoc.................................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

datatree/tests/test_dataset_api.py:166:80: E501 line too long (87 > 79 characters)
...
(py38-mamba) tom@pop-os:~/Documents/Work/Code/datatree$ git status
On branch linting
Your branch is up to date with 'origin/linting'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   .pre-commit-config.yaml

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	.idea/
	datafiles/
	datatree/accessors.py
	design_notes.md
	setup.cfg

no changes added to commit (use "git add" and/or "git commit -a")

(there are more flake8 and mypy suggestions/warnings later but I'll try and deal with them afterwards)

@TomNicholas TomNicholas added the CI Continuous Integration and packaging tools label Aug 25, 2021
@jhamman
Copy link

jhamman commented Aug 25, 2021

@TomNicholas - try merging #22 into your linting branch and running pre-commit over all the files again.

@TomNicholas
Copy link
Member Author

@jhamman that works nicely, thanks!

@jhamman
Copy link

jhamman commented Aug 25, 2021

Nice! The remaining failure seems to be related to mypy. We could turn that off for now, or you can make those fixes here.

@TomNicholas
Copy link
Member Author

TomNicholas commented Aug 25, 2021

The remaining failure seems to be related to mypy. We could turn that off for now, or you can make those fixes here.

Yes - I'm currently trying to fix the mypy failures, but I don't understand a lot of them. What I think we could do is turn off mypy for now in the CI and open a separate PR to discuss changes needed to make mypy pass locally.

@jhamman
Copy link

jhamman commented Aug 25, 2021

Great. Just comment out the my-py section in the pre-commit-config and you should be good to go.

@TomNicholas TomNicholas merged commit 085fc5d into main Aug 25, 2021
@TomNicholas TomNicholas deleted the linting branch August 25, 2021 16:40
flamingbear pushed a commit to flamingbear/rewritten-datatree that referenced this pull request Jan 19, 2024
* black reformatting

* add setup.cfg to configure flake8/black/isort/mypy

* add setup.cfg to configure flake8/black/isort/mypy xarray-contrib/datatree#22

* passes flake8

* disabled mypy for now

Co-authored-by: Joseph Hamman <jhamman@ucar.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration and packaging tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants