search

eurec4a / eurec4a-intake

Intake catalogue for EUREC4A field campaign datasets
17 stars 19 forks source link

add ipfsspec install to README #31

Closed leifdenby closed 11 months ago

leifdenby commented 3 years ago

To access the JOANNE dataset ipfsspec is now needed. This just makes sure that it is included in the README as a package that must be installed

d70-t commented 3 years ago

I am wondering if it is a good idea to try to keep the README in sync with the requirements manually (and possibly incomplete) or if it is better to suggest

pip install -r requirements.txt

in the README.md

leifdenby commented 3 years ago

As written, I'd consider to reference the requirements.txt in stead, but I don't want to halt the PR entirely.

I thought about that. But that requires people to clone the repository so that they have requirements.txt available locally. What's nice with putting it in the README is that people can simply install the packages with pip and then open the catalog directly from github (through the raw github URL as I put in the readme)

d70-t commented 3 years ago

That's a valid argument. However, I just checked if I could directly install via a remote requirements file and it seems to be working just fine. So what do you think about recommending the following in the README?

pip install -r https://raw.githubusercontent.com/eurec4a/eurec4a-intake/master/requirements.txt
d70-t commented 3 years ago

So I can't find this behaviour of pip documented somewhere, but I traced it a bit through the source code of pip.

The requirements file parser uses get_file_content() to read the contents of a file when asked to do so. get_file_content() handles urls as well as local files. The function has been moved around a bit during pip development, but I could at least trace it back to 2 Jul 2010 where it seemingly had been able to access remote files as well as it does now. Thus, I'd argue that we can at least be pretty sure that this will work in about all pip versions around.

An argument against this command would maybe be that it looks unfamiliar and maybe scary?

leifdenby commented 3 years ago

An argument against this command would maybe be that it looks unfamiliar and maybe scary?

Yes, I agree :) I do think we should at least add ipfsspec to the README as I expect most people won't be cloning the repository and installing with pip. Maybe we could suggest that people check the requirements.txt file in case something isn't working? Or we encourage them to raise and issue?

d70-t commented 3 years ago

Ok, then let's go for that solution: mentioning ipfsspec + recommending to have a look into requirements.txt + raising an issue if things are not working.

leifdenby commented 3 years ago

Great, I'll add that to this pull-request

RobertPincus commented 3 years ago

@leifdenby Seems to me this PR is out of date, as we now have a requirements.txt, and might be closed or?