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

Add python steps to Github Workflow (#47) #51

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Add python steps to Github Workflow (#47) #51

merged 2 commits into from
Nov 28, 2022

Conversation

blms
Copy link
Collaborator

@blms blms commented Nov 23, 2022

In this PR

Per #47:

  • Slightly reorganize convertJSON.py to reorder imports, move color generation into its own section, and dynamically create the list of colors based on the number of authors
  • Add python setup, pip packages installation, and convertJSON run to github workflow

@blms blms requested a review from rsimon November 23, 2022 18:08
Copy link
Collaborator

@rsimon rsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Happy for this to be merged. If you want to tweak the GH Action, we could try this before merging. Otherwise we can also leave it for later.

- name: Regenerate data files from CSV
working-directory: ./script
run: python convertJSON.py

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's possible to trigger the Python rebuild only if the CSV data changes? I have never done this (and my knowledge of Github Actions is fairly beginner level...) But perhaps this is something that could be achieved through the paths directive?

https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-including-paths

E.g. something like?

on:
  push:
    branches:    
      - main
    paths:
      - '*.csv'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I think it's not possible using that syntax, since that only decides whether or not the workflow should be run. We could instead do something like this: https://stackoverflow.com/a/70711156/20382305

However, I think that's a bit complicated for something that wasn't requested directly by the client. But feel free to open an enhancement-tagged issue with that idea!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - good point. Yes, this looks like it's going to either run the action as a whole, or not at all. I wonder what happens if we were to create two separate actions: one for the Python data conversion, one for the GH pages build. But that would probably get too messy & we might end up debugging GitHub Action race conditions... I'd say let's leave it as is, and revisit if there's ever explicit client demand.

@blms blms merged commit e52f914 into main Nov 28, 2022
@blms blms deleted the 47-github-workflow branch November 28, 2022 14:53
@blms blms mentioned this pull request Nov 28, 2022
1 task
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.

None yet

2 participants