Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Python page mostly ok #33

Merged
merged 15 commits into from
Oct 29, 2021
Merged

Python page mostly ok #33

merged 15 commits into from
Oct 29, 2021

Conversation

Clonkk
Copy link
Member

@Clonkk Clonkk commented Oct 27, 2021

No description provided.

@Clonkk
Copy link
Member Author

Clonkk commented Oct 27, 2021

PR fails probably because The python envronment existing does not have scipy / numpy installed :/.

Anyone have a solution to this ? Should we just add Python package to the CI ?

@HugoGranstrom
Copy link
Member

Anyone have a solution to this ? Should we just add Python package to the CI ?

I guess so, that must be how all Python people do their CIs, no? 🤷 pip install numpy and we are go perhaps

@pietroppeter
Copy link
Contributor

Nice addition!

For Python CI we could use the actions to setup Python, here there are many recipes: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python

The simplest is probably the one with specific Python version and install of requirements:


steps:
- uses: actions/checkout@v2
- name: Set up Python
  uses: actions/setup-python@v2
  with:
    python-version: '3.x'
- name: Install dependencies
  run: |
    python -m pip install --upgrade pip
    pip install -r requirements.txt

Python requirements should be provided with the usual requirements.txt

@Clonkk
Copy link
Member Author

Clonkk commented Oct 27, 2021

It seems pip install numpy scipy is enough. I'll give it a second read tomorrow and if everything's it can be reviewed / approve

@HugoGranstrom
Copy link
Member

Nice! I guess @pietroppeter 's suggestion would be the way to go in the future, but just to get it working the pip install works fine

@Clonkk
Copy link
Member Author

Clonkk commented Oct 28, 2021

Someone needs to approve in order to merge

Copy link
Member

@Vindaar Vindaar left a comment

Choose a reason for hiding this comment

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

Only some small changes.

Aside from that thanks a lot! 🥳

Clonkk and others added 8 commits October 28, 2021 19:49
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
Co-authored-by: Vindaar <basti90@gmail.com>
@Vindaar
Copy link
Member

Vindaar commented Oct 28, 2021

Fine by me now.

Copy link
Member

@HugoGranstrom HugoGranstrom left a comment

Choose a reason for hiding this comment

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

Nice work! Just a typo and a note to add some text in the julia file if you are not going to add it yet 😀

Clonkk and others added 2 commits October 29, 2021 09:45
Co-authored-by: Hugo Granström <5092565+HugoGranstrom@users.noreply.github.com>
@Clonkk Clonkk merged commit 60ca888 into main Oct 29, 2021
@Clonkk Clonkk deleted the clonkk_page_language_integration branch October 29, 2021 08:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants