Tools#3
Conversation
ScottSoren
left a comment
There was a problem hiding this comment.
These tools look great!
I'll admit that I haven't got it running yet, but may as well get these comments to you anyway before I disappear for meetings the rest of the afternoon.
Most of the comments are questions which are more for my understanding than anything else but may as well get answers in the documentation.
My main concern about it is whether it adds a barrier of complexity that deters experimentalists who are not software developers from contributing. That is not a critique of the tools, but just a note that I think we have to make ixdat (or at least parts of ixdat) develop-able without the tools. It's more a governance thing, who's responsibility is it to do QA and when, and a luxury problem for a hypothesized time in the future when we have lots of contributers.
| @@ -0,0 +1,8 @@ | |||
| [flake8] | |||
| max-line-length = 95 | |||
There was a problem hiding this comment.
I've been using 89, but okay. I think no matter what I use, I'll always often wish it was a few chars more.
There was a problem hiding this comment.
THAT is 100% and totally at the discretion of the project BDFL (https://en.wikipedia.org/wiki/Benevolent_dictator_for_life). Line length is something that people get religious about and I don't want to spend 2 seconds discussing it. I wrote 95 here, because in other projects we have been going with recommendations that have been mentioned in a few different places, which is aim for 90 but allow up to 95, which essentially means that the developer should break at 90 unless there is a good reason, like a long link or something like that, which can be finished in the last 5. But since linters aren't that intelligent, it needs to allow up to 95 if that is the policy. But I will put there whatever you say.
ScottSoren
left a comment
There was a problem hiding this comment.
Okay, I tried to follow the instructions in tools.rst to get up and running, and find they are not complete, at least not for Windows with Anaconda Python and PyCharm.
There are comments with details.
I will keep playing around and update here if I get it working.
|
|
||
| First we should locate the path of the tools. In case a separate | ||
| virtual environment is used, located e.g. in c:\venv\ixdat, then | ||
| c:\venv\ixdat\Scripts will be the path of tools. TODO anaconda. |
There was a problem hiding this comment.
I'm using anaconda and get the following, in the PyCharm Ipython console:
>>> import sys
>>> sys.executable
'C:\\ProgramData\\Anaconda3\\envs\\ixdat\\python.exe'
so...
/c/ProgramData/Anaconda3/envs/ixdat/python.exe
?
There was a problem hiding this comment.
It's not working for me. After adding the line to ~ /.bashrc and sourcing ~/.bashrc, it's there:
$ echo $PATH
/c/ProgramData/Anaconda3/envs/ixdat: ....
but invoke remains unrecognized by git.
I did remember to install invoke in ixdat's python environment in PyCharm (pip install invoke in Anaconda only gets it onto its (base) environment).
So I think some things are missing in these instructions.
There was a problem hiding this comment.
Yeah. I'm slowly learning that while anaconda is really good for getting a lot of high quality, high speed scientific code into the hands of scientists in a hurry I find that it is actually somewhat harder to figure out exactly what goes on. I actually intended to update this later specifically for anaconda (since I just uninstalled it in favor of an ordinary python installation and virtual environments, but since you show an interest in the hooks I think I will go back to get this to work and properly documentet.
There was a problem hiding this comment.
Actually, can I persuade you do me one favor. Can I get you to run:
echo %path%in your anaconda prompt and write the results here.
There was a problem hiding this comment.
Great! :|
I will see about re-installing anaconda shortly, but it will delay this PR a little further. I would prefer actually to merge it and fix it later as time allows (since it is marked with a TODO in TOOLS.rst) but I will leave that up to you.
There was a problem hiding this comment.
That's fun. That command is mentioned like ... everywhere. Grumble grumble.
Also added a --single option to the tox invoke task and updated the hooks files to use that option.
|
Thanks for the review @ScottSoren Ok, so I resolved most of the issues with a few pending feedback from you @ScottSoren. There are 2 things I wanted to comment in directly: 1 What I suggest as the next step is to enable a continuous integration service, which will display a CI status (right besides the "merge-ability" status at the bottom of every PR, like so: Now, I would suggest enabling the display of such status no-matter what, so that especially we, but also the interested new contributor, can see the CI status straight away (as a maintainer you have to reduce your manual chores as much as possible, trust me). However, whether an all green status is required for merge or not, is a decision. And we can set github up for the archive in a manner that reflects that decision. If you want to insist that all is green before merge, we can ask github to enforce that, but if you want to allow for merging "dirty" and then fixing the QA issues manually afterwards yourself (which is a policy I have seen some places), then we can just choose not to have github enforce it, in which case the CI status will be just for information. 2 |
|
Looks great! |
|
Merged these tools! Thanks @KennethNielsen. |
Revert "temperature-pressure MS plotting"


This PR introduces parts of my standard python development tool chain into ixdat for evaluation. The tools falls into two different categories, "tools" and something which I generally call "tool runners".
Of tools this introduces only
pytestas a test runner andflake8as a linter (takes care of some coding errors and coding style related stuff). flake8 is more or less standard if you want a fast and not too opinionated linter and I have over time come to prefer that over something like pylint. I did not includeblackto the configuration just yet, as I wanted to see if you are happy with it first.Tool runners is likely the more interesting category, because those types of tools may not be too well known for scientists. These are tools to run your tools and perform tasks for you. You may ask, "why would I want a tool to run a tool?". The answer is that it can help you remember worthwhile options and in general make sure that it is run in an consistent manner and that tasks are performed easier and consistently.
As tools runners I have introduced
invoke(http://www.pyinvoke.org/). The purpose of invoke is to easily make it possible and easy to do all the things/perform all the tasks, you need during development. Whenever you need to do something, you should check if invoke has a task for it and ask it to do it. If you perform a task that invoke hasn't been configured for and you do it more than twice, you should probably just go ahead and define an invoke task for it. In the olden days Unix developers would often use something like makefiles for this (https://en.wikipedia.org/wiki/Makefile), but they don't work on Windows which is the reason something like sphinx has both a makefile and a make bat file to compile the docs. Invoke aims to bridge that gap by providing as much of a platform independent command execution environment available as possible and by making its configuration file a pure python file, which means that you can yourself program your way around the whatever platform difference issues you may run into.During development invoke commands to commonly use could be something like:
to run the linter
to run the tests
to run all the standard code checks (lint and test for now)
to delete all the temporary and cache files
to build the docs (not yet implemented)
to see a list of all available tasks do:
The second tool runner is
tox(https://tox.readthedocs.io/en/latest/). tox is a tool to run all your QA tools in as consistent a manner as possible across as many of the supported python version as possible. It runs the QA tools in a virtual environment on a freshly installed version of your code. Because tox is meant mainly for continuous integration environments, it will try and perform all the tests on all the supported python versions. If they are unavailable, it will report an error, for this purpose I have configured an invoke task to run tox only on the python version that are available.To run tox purely do:
to run in parallel do:
To run tox only on available python versions do
I tried to gather all the information that developers will needs about the tools in the TOOLS.rst file. Please give that a look.
NOTE: This is WIP since I know it has a directory conflict with #2, which I will resolve here when #2 is merged.
EDIT: I completely forgot to mention that the PR also includes a git hook, which is a mechanism to run tests and checks before pushing commits to a remote branch. Please see the details of it and installation instructions in TOOLS.rst.