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

Update the python setup instructions #442

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kappe-c
Copy link

@kappe-c kappe-c commented Sep 17, 2024

Just minor things. See the commit messages.

What was `>3.11.x` even supposed to mean? I think greater than *or equal* is more straightforward than this formulation with some variable x.
I have tested this simpler setup (as given on the main readme) myself.
I know at least three Linux distributions where `py` is not available by default. If someone knows how to set up the shorter alias or symbolic link, they know that they can use it here. The longer command in our examples may lead to fewer frustrated users.
Different to the previous commit, for MD code blocks `py` is enough. Also, you hardly ever need a semicolon in python.

Of course you can replace the command `py` with anything that leads to the python executable of your liking.
1. Install [python >=3.12](https://www.python.org/downloads/)
2. Run `pip install arctrl`
Copy link
Member

Choose a reason for hiding this comment

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

Does the reference to fsspreadsheet work?

Copy link
Author

Choose a reason for hiding this comment

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

Actually, later I had to do pip install fsspreadsheet because python complained about not finding such a module. But I think it should in fact be possible to stick with a "one-step installation". Pip should be able to handle requirements without the user having to deal with it manually.

Copy link
Member

Choose a reason for hiding this comment

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

I think the problem here is, that theres not actually a dependency to fsspreadsheet (the python package). So pip can't know this.

Copy link
Author

Choose a reason for hiding this comment

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

Could a solution be to add "fsspreadsheet" to pyprojec.toml (in the root folder of this project) -> [build-system] -> requires ?


Thats it! 🎉

You can now reference ARCtrl in any `.py` file and run it with `py path/to/Any.py`.
You can now reference ARCtrl in any `.py` file and run it with `python path/to/Any.py`.
Copy link
Member

Choose a reason for hiding this comment

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

This can vary by environment, which would be the more common one?

Copy link
Author

Choose a reason for hiding this comment

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

Tbh, I never saw a simple py before. If anything, sometimes python3 is/must be used, in my experience.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, okay doesn't really matter to me. The developer will hopefully pick the right one for himself.


```python
Copy link
Member

Choose a reason for hiding this comment

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

Does this make a difference in your markdown editor?

Copy link
Author

Choose a reason for hiding this comment

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

No but it is shorter. And for this markup purpose I promote the short versions. They are also less ambiguous regarding the spelling. I (and not only I) argue that for both humans and machines language tags like the following are actually easier and better for compatibility: py for Python, js for JavaScript, sh for shell/bash, etc.


Verify correct setup by creating `ARCTest.py` file with the content from below in the same folder, which contains your `requirements.txt`. Then run `py ./ArcTest.py`. This will print `<class 'arctrl.arc.ARC'>` into the console.
Verify a correct setup by creating a file `ARCTest.py` (anywhere). Then run `python path/to/ArcTest.py`. This will print `<class 'arctrl.arc.ARC'>` into the console.
Copy link
Member

Choose a reason for hiding this comment

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

Agree with the "anywhere" and "python path/to" part. But the first sentence feels a bit off now grammatically. How about "Verify correct setup by creating ARCTest.py file (anywhere).

Copy link
Author

Choose a reason for hiding this comment

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

Idc tbh :-) The sentence was in bullet point form anyway. Feel free to edit the style as you see fit. Your suggestion is fine by me.

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.

2 participants