robertmartin8 / PyPortfolioOpt

Financial portfolio optimisation in python, including classical efficient frontier, Black-Litterman, Hierarchical Risk Parity
https://pyportfolioopt.readthedocs.io/
MIT License
4.51k stars 956 forks source link

Add Windows CI coverage #277

Closed phschiele closed 3 years ago

phschiele commented 3 years ago

@robertmartin8 As discussed in #273, this PR adds Windows coverage to the CI.

Some remarks:

robertmartin8 commented 3 years ago

@phschiele Packaging has always been an area for improvement. There are actually four dependency specifications:

  1. pyproject.toml
  2. requirements.txt
  3. pipfile
  4. setup.py

pyproject.toml is the one that I actively update and consider to be the ground truth. For the sake of best practices, it is what I recommend people use.

requirements.txt is important for people who want to use the code locally (though pip install -e is probably the best way to go). When I was setting up poetry, I couldn't find a way to create requirements from pyproject, but I'd love to be able to do that.

I added pipfile reluctantly in response to #71. I really don't know much about pipenv.

I think setup.py can be useful to have as a fallback – when other installation methods fail, cloning then python setup.py can often be a robust last resort. The key point about setup.py is that it supports python 3.5, for users who need it (though as we discussed recently, I have dropped official support).

phschiele commented 3 years ago

@robertmartin8 Thanks for the detailed explanation. I think it makes sense to have pyproject.toml as the source of truth.

I am not an expert on packaging, and often read conflicting suggestions for best practices. The requirements for the packaging seem to be:

I was in the process of writing my views on the current file types present, but the more I tried out locally, the more obscure it got. There are some ways to generate (some of) these files directly from poetry, like:

poetry export -f requirements.txt --output requirements.txt --without-hashes

or

poetry build
tar -xvf dist/*.tar.gz --wildcards --no-anchored '*/setup.py' --strip=1

This should be enough to cover most cases (I think pipenv also has an install -e . option, which works with setup.py). Not sure if these automatic conversions are ideal, though.

I eventually found dephell which seems to be a quite handy tool for such tasks. After installing (curl -L dephell.org/install | python3), I was able to generate all three fallback files, which looked quite good:

dephell deps convert --from pyproject.toml --to requirements.txt
dephell deps convert --from pyproject.toml --to Pipfile
dephell deps convert --from pyproject.toml --to setup.py

Sadly, the tool seems to be no longer actively developed.

So in summary, I think it would be ideal to have one config file. If that's not possible for compatibility reasons, the second-best option would be to have the same contents in all config files, enforce by a single small script that could be added to the development/publishing workflow.

Do you think these experiments go into the right direction?

robertmartin8 commented 3 years ago

@phschiele dephell looks prety cool. I'll try and use that to generate requirements.txtand pipfile (though requirements.txt is much more important to me than pipfile).

As for setup.py, I will leave it the same for the poor py3.5 users out there – life must be hard enough for them!