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

Pycodestyle heavily failing #105

Closed
JosePizarro3 opened this issue May 16, 2023 · 17 comments
Closed

Pycodestyle heavily failing #105

JosePizarro3 opened this issue May 16, 2023 · 17 comments
Labels
pipeline fails CI/CD pipeline failing (mainly due to pycodestyle fails)

Comments

@JosePizarro3
Copy link
Collaborator

@ladinesa @ndaelman-hu , the pipeline is failing with a lot of fails regarding pycodestyle in some parsers. Should this be fixed?

@JosePizarro3 JosePizarro3 added the pipeline fails CI/CD pipeline failing (mainly due to pycodestyle fails) label May 16, 2023
@ladinesa
Copy link
Collaborator

ladinesa commented May 16, 2023

This is the very reason why I always remind running pylint and pycodestyle locally. Not sure who will take this over. Maybe look at the list of errors and if you think you are responsible please commit the necessary fixes.

@ndaelman-hu
Copy link
Contributor

Will solving this make the pipeline pass? If so, we should look into this, else this can come later imo.

@JosePizarro3
Copy link
Collaborator Author

This is the very reason why I always remind running pylint and pycodestyle locally.

That's why I created the label 😆

Will solving this make the pipeline pass? If so, we should look into this, else this can come later imo.

Nope, but at least we should be reaching pylint, without pycodestyle errors, right?

@ndaelman-hu
Copy link
Contributor

Nope, but at least we should be reaching pylint, without pycodestyle errors, right?

Sure, but it's very low-priority. I can take it, but next week at the earliest.

@ladinesa Any update on the other tests? When could we start implementing them again?

@ladinesa
Copy link
Collaborator

Will solving this make the pipeline pass? If so, we should look into this, else this can come later imo.

The thing is, we are updating the metainfo quite frequently and the nomad release cannot keep up. I will update the nomad lab package in the github workflow but this may not necessarily make the tests pass if the metainfo defs version is not update to date.

Again, please run pylint, pycodestyle and pylint locally. It is harder to fix when these errors add up.

@ladinesa
Copy link
Collaborator

ladinesa commented May 16, 2023

I think addressing pylint and pycode should not be secondary. @ndaelman-hu not sure what you mean update on the other tests. If you mean, when the pylint will pass in github tests and run the tests, I do not think this will ever happen because as I have said we are updating nomad frequently and releases are not done every update. We discussed this months ago and I have warned you of this exact situation.

@JosePizarro3
Copy link
Collaborator Author

Maybe a very dumb question: is it possible to skip pylint in the github actions? Or skip it + include a message saying "Please run pylint locally with: ....."?

Seeing that we are never going to be able to pass that part of the pipeline, maybe we should do it like this?

@ladinesa
Copy link
Collaborator

Maybe a very dumb question: is it possible to skip pylint in the github actions? Or skip it + include a message saying "Please run pylint locally with: ....."?

Seeing that we are never going to be able to pass that part of the pipeline, maybe we should do it like this?

The tests will not run because you are using an outdated nomad package.

@ndaelman-hu
Copy link
Contributor

@ladinesa Yes, these are the tests that I meant. I mostly remember there being a problem with a package, that's why I was asking.

I understand your point regarding the metainfo though. Do you think that the NOMAD installation could be automated?
On Gitlab, the CI/CD produces a docker image, maybe Github could pull it? Just an idea.

I think addressing pylint and pycode should not be secondary.

No, I get you, but it's fine if we fix it next week, right? I think we're all oversaturated with other tasks atm.

@ladinesa
Copy link
Collaborator

Actually, you can manually trigger release for your commit, but I think you should ask Markus if you can do this. It is not part of the commit workflow because we do not want to clutter the nomad releases.

@ladinesa
Copy link
Collaborator

I managed to fix the issues and also the github workflow. The develop branch should always pass now.

@ndaelman-hu
Copy link
Contributor

So, the current develop has no linting errors? Many thx!

@ndaelman-hu
Copy link
Contributor

@ladinesa Could you please bump up the pylint version? Currently, it's at 2.3.0, which has a bug in it with Union. This is making me fail the pipeline. The minimally required version is 2.6.1, but I'd prefer the same one as on Gitlab.

@ndaelman-hu ndaelman-hu reopened this May 22, 2023
@ndaelman-hu
Copy link
Contributor

@ladinesa Could you please bump up the pylint version? Currently, it's at 2.3.0, which has a bug in it with Union. This is making me fail the pipeline. The minimally required version is 2.6.1, but I'd prefer the same one as on Gitlab.

@ndaelman-hu ndaelman-hu reopened this May 22, 2023
@ndaelman-hu
Copy link
Contributor

ndaelman-hu commented May 22, 2023

Moreover, could we pls also swap the order of the tests to that in Gitlab? i.e.

  1. pytest
  2. pycodestyle
  3. mypy
  4. pylint

Out of these, pytest is the most informative of all. Meanwhile, fixing the linting it something that one can do at the very end, before merging. pylint, on the other hand, is the slowest of all the linting tests, so it can go last.

@ladinesa
Copy link
Collaborator

I updated pylint and added directive to always run pytest. You can always run theses tests locally if you need quick feedback.

@ndaelman-hu
Copy link
Contributor

ndaelman-hu commented May 23, 2023

Many thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pipeline fails CI/CD pipeline failing (mainly due to pycodestyle fails)
Projects
None yet
Development

No branches or pull requests

3 participants