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

Initial implementation of Flake8 action #101

Merged
merged 32 commits into from
Jun 4, 2024

Conversation

ericdrosas87
Copy link
Contributor

Initial implementation of Flake8 action

Initial implementation of Flake8 action
@ericdrosas87
Copy link
Contributor Author

Resolves #100

Explicitly setting other package validation to false
Confirming config file is created as expected
Fixed config file creation issue
Trying out a relative path for config file
Moved config file to home dir with cat
Seeing if a `.flake8` file in the repo will be picked up
Fixed Flake8 validation errors in `utils.py`
Fixed more validation errors
Adding explicit jupyter notebook run
Added separate step to run notebook validation
Trying new Github action
Adding back .py validation
Split out py script and notebook validation to their own jobs
@ericdrosas87 ericdrosas87 requested a review from beckynevin May 9, 2024 21:23
Excluding `experimental_notebooks` folder in Flake8 config
@ericdrosas87
Copy link
Contributor Author

ericdrosas87 commented May 9, 2024

@beckynevin We'll need to figure out what to do about the notebooks in the experimental_notebooks folder. As far as I can tell, the action added by this PR is the only Flake8 Action that can also validate Jupyter Notebooks. I confirmed with the maintainer of the Action that there's no way to ignore/exclude a subset of notebooks, so the Action will parse the entire repo for any .ipynb files, including the experimental notebooks and unless we bring them up to snuff will continue to throw off validation.

There are a few options:

  1. Move the experimental notebooks out of this repo into their own repo
  2. Keep them on their own branch and remove them from the main branch
  3. Change the file extension to something like ipynbx so they aren't picked up by the Action
  4. Make the experimental Flake8 compliant

Let me know your thoughts when you have a moment.

@beckynevin
Copy link
Collaborator

@beckynevin We'll need to figure out what to do about the notebooks in the experimental_notebooks folder. As far as I can tell, the action added by this PR is the only Flake8 Action that can also validate Jupyter Notebooks. I confirmed with the maintainer of the Action that there's no way to ignore/exclude a subset of notebooks, so the Action will parse the entire repo for any .ipynb files, including the experimental notebooks and unless we bring them up to snuff will continue to throw off validation.

There are a few options:

  1. Move the experimental notebooks out of this repo into their own repo
  2. Keep them on their own branch and remove them from the main branch
  3. Change the file extension to something like ipynbx so they aren't picked up by the Action
  4. Make the experimental Flake8 compliant

Let me know your thoughts when you have a moment.

You fixed this right? Because I currently don't see them being reviewed in the action.

@beckynevin We'll need to figure out what to do about the notebooks in the experimental_notebooks folder. As far as I can tell, the action added by this PR is the only Flake8 Action that can also validate Jupyter Notebooks. I confirmed with the maintainer of the Action that there's no way to ignore/exclude a subset of notebooks, so the Action will parse the entire repo for any .ipynb files, including the experimental notebooks and unless we bring them up to snuff will continue to throw off validation.

There are a few options:

  1. Move the experimental notebooks out of this repo into their own repo
  2. Keep them on their own branch and remove them from the main branch
  3. Change the file extension to something like ipynbx so they aren't picked up by the Action
  4. Make the experimental Flake8 compliant

Let me know your thoughts when you have a moment.

This looks like it was resolved, right? I can't quite tell when I'm looking at the validate action for notebooks which notebook it's looking at but it looks like you solved this and now it's just looking within the main folder right?

ericdrosas87 and others added 5 commits May 16, 2024 14:39
Upped `mhitza/flake8-jupyter-notebook` action minor version number
Trying glob pattern in `.flake8` exclude prop
Explicit `exclude` paths in `.flake8`
@ericdrosas87 ericdrosas87 merged commit b3e3dae into main Jun 4, 2024
2 checks passed
@ericdrosas87 ericdrosas87 deleted the 100-add-flake8-action-workflow branch June 10, 2024 19:41
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.

Set up Github Action workflow for running Flake8 automatically upon PR creation
2 participants