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

Software sustainability #41

Closed
3 of 5 tasks
bhigy opened this issue Nov 26, 2020 · 12 comments
Closed
3 of 5 tasks

Software sustainability #41

bhigy opened this issue Nov 26, 2020 · 12 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request software sustainability

Comments

@bhigy
Copy link
Contributor

bhigy commented Nov 26, 2020

Define actions still required by the software sustainability plan and implement them.

@bhigy
Copy link
Contributor Author

bhigy commented Dec 1, 2020

@cwmeijer and @egpbos, can one of you look into this (in particular the 3rd point)?

@bhigy bhigy added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 1, 2020
@egpbos
Copy link
Collaborator

egpbos commented Dec 2, 2020

Wow, this is the first time I read these CII criteria, very exhaustive, nice!

Under "Warning flags" it basically says we should use a linter in strict mode. I think this can indeed be very useful. I once did a project with pylint at maximum strict mode. At first it's very annoying, but in the end I was very happy with the result, even though I sometimes disagree with the defaults. Strict mode also notifies you that you should write doc-strings for all functions and modules. This way we would automatically satisfy the documentation criteria.

Of course, we don't want to get into holy formatting wars ;) But I think we are actually already very close in the way we write code. I would say that we just configure pylint to ignore the things we still turn out to disagree on, which is easy enough to do.

Then we can also add a linting check to CI. I would also then activate the linter in my editor, so that it becomes easy to fix some warnings along the way (i.e. while you may be doing something else).

@bhigy
Copy link
Contributor Author

bhigy commented Dec 2, 2020

Sounds good to me. It will also force me to finally set the linewidth properly. I still get warnings when the line exceeds 80 characters, while I think we once talked about setting that to 120.

@bhigy
Copy link
Contributor Author

bhigy commented Dec 2, 2020

Wow, this is the first time I read these CII criteria, very exhaustive, nice!

This comes from the software sustainability plan that the eScience center asked us to fill. 😃 I didn't know about that either.

@egpbos
Copy link
Collaborator

egpbos commented Dec 2, 2020

Ahaha, right, clearly I wasn't involved in that ;)

@egpbos
Copy link
Collaborator

egpbos commented Dec 2, 2020

Yeah, I personally tend to set it to 120, but I have no issues making it bigger still, or ignoring the rule entirely. There are a lot of rules that are far more useful than this :)

@bhigy
Copy link
Contributor Author

bhigy commented Dec 4, 2020

Other point we should discuss I think is the versioning of the code. I would like to add a DOI to the repository through Zenodo, which can automatically archive the different releases of the code. Making such releases on a consistent basis would be good (we have one so far).

@egpbos
Copy link
Collaborator

egpbos commented Apr 28, 2021

With commit 54fc340, I fixed the LabelEncoder warning (code quality improvement, CII section "Warning flags").

@egpbos
Copy link
Collaborator

egpbos commented Apr 28, 2021

Another warning fix should hopefully come from wandb/wandb#2129

@egpbos
Copy link
Collaborator

egpbos commented May 10, 2021

Wandb PR has been merged, so that's another warning gone after updating to wandb 0.10.30.

@egpbos
Copy link
Collaborator

egpbos commented May 17, 2021

All Python warnings from running the experiments are gone now, also #90.

A next logical step would be to "fix" all current linter warnings produced by flake8. After that, we could see what pylint on extreme strictness levels suggests, maybe it has some useful ideas.

@egpbos
Copy link
Collaborator

egpbos commented May 18, 2021

Ok, made a PR for a first run of flake8 fixes: #91.

For next steps, we should probably make a list of things we need to do to fulfill the CII criteria. Let's start out just copying it and striking through items we have already. I'll do this in a new issue to keep this one from becoming too long and messy.

In fact, I'll just go ahead and split up everything I think is still unresolved from this thread into separate issues, so we can close this and continue there:

@egpbos egpbos closed this as completed May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request software sustainability
Projects
None yet
Development

No branches or pull requests

2 participants