Skip to content

run by run v3 with font removed#526

Merged
marrbnl merged 9 commits intostar-bnl:mainfrom
ssedd1123:main
May 10, 2023
Merged

run by run v3 with font removed#526
marrbnl merged 9 commits intostar-bnl:mainfrom
ssedd1123:main

Conversation

@ssedd1123
Copy link
Copy Markdown
Contributor

It should be pure python code now.

@starsdong
Copy link
Copy Markdown
Member

Any comment from your side, Grigory and Zach? Thanks

Comment thread StRoot/PWGTools/BadRunQA/README.md Outdated

To install the software, follow these steps:
The EASY way (Linux only. For Windows and Mac, use the recommended way):
1. Run the following command: `python3 -m pip install --user matplotlib ruptures pandas numpy scipy pyfiglet uproot scikit-learn`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Usually it helps to freeze the dependencies in requirements.txt (or in pyproject.toml). It should help with reproducibility of results as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have updated my repo to include requirements.txt for Linux. For some reasons the packages must be installed with anaconda on Windows so I don't have one for Windows.

I have also updated my run-by-run code to include a legacy mode to emulate the behavior of the old version (v2).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The requirements.txt file is platform-agnostic and used by Python package managers to establish dependencies in virtual environments. However, it seems to have been left out of this particular pull request.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried to use requirements.txt on my Windows desktop and Windows laptop with pip3 install -r requirements.txt and the package "rupture" gives me errors. Something along the line of "error: Microsoft Visual C++ 14.0 or greater is required. Get it with "Microsoft C++ Build Tools"". I did update my Microsoft C++ Build Tools but it's still not working. I can still get that particular package to work on Windows, but only if I install with conda, i.e. conda install ruptures.

However, this requirements.txt definitely works on RCAS.

@ssedd1123 ssedd1123 requested a review from marrbnl May 4, 2023 00:51
Chun Yuen Tsang added 2 commits May 4, 2023 21:21
@marrbnl
Copy link
Copy Markdown
Member

marrbnl commented May 9, 2023

@plexoos Hello Dmitri. This pull request has been approved by two people, so I think it is ready to be merged. Since I have not done this before, should I click "Squash and merge" or something else?Thanks.

@starsdong
Copy link
Copy Markdown
Member

Rongrong, yes. Generally, I would wait for the CI build and test jobs to finish successfully. Then the "Squash and merge" button will turn green. I guess it is not relevant for this PR anyway. We can merge now if it is urgent or wait for the rest to finish.

@marrbnl
Copy link
Copy Markdown
Member

marrbnl commented May 9, 2023

Thanks for the clarification, Xin. We can wait for the test to finish.

@marrbnl marrbnl merged commit 5a21185 into star-bnl:main May 10, 2023
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.

4 participants