Chavithra / degiro-connector

This is yet another library to access Degiro's API.
BSD 3-Clause "New" or "Revised" License
220 stars 48 forks source link

Maximum allowed pandas version is almost 1 year old #29

Closed fernandobrito closed 2 years ago

fernandobrito commented 2 years ago

Hi! 👋

This project requires pandas <= 1.1.5: https://github.com/Chavithra/degiro-connector/blob/main/pyproject.toml#L26

Is there any particular version of why it needs to be <= 1.1.5? That version is almost 1 year old, and it would be great to bump the requirement to allow newer versions (1.3.x) if there are no known issues.

Chavithra commented 2 years ago

Hi,

Reason This is just to allow compatibility with python 3.6.

Solutions I think if you apply these two commands you will have both degiro-connecor and a newer version of pandas :

pip install degiro-connector
pip install -U pandas

Another solution : change the required pandas version inside a fork and pip install this fork :

pip install git+https://github.com/chavithra/degiro-connector.git
fboerman commented 2 years ago

@Chavithra while I understand you want to support older versions, python3.6 will be deprecated by the end of this year, almost all distros have long updated their python version.

Please consider dropping its support so updated pandas version is installed when you install the package.

Chavithra commented 2 years ago

Alright, you guys convinced me.

Made a new release that supports python 3.7.

Plus you should be able to use newer versions of pandas.

fernandobrito commented 2 years ago

Since I'm adding your library as a dependency of my library and installing my library through pip, I'm not sure how the pip -U would work in that case.

Great, thanks for the quick action on bumping the pandas requirement! I will give it a try this weekend and most likely close this issue. 🙌

fernandobrito commented 2 years ago

Awesome, I confirm that I can now have degiro-connector alongside with the latest pandas version as dependencies of my project. Thanks for the quick action!

fernandobrito commented 2 years ago

Hmm, wait. Actually, I do confirm that my code works with pandas 1.2.x, but when I try 1.3.x (latest) I get an error when I try to call ChartHelper.serie_to_df.

This line https://github.com/Chavithra/degiro-connector/blob/main/degiro_connector/quotecast/actions/action_get_chart.py#L197 returns

ValueError: DataFrame constructor not properly called!

On 1.3.x. Let me try to reproduce it on a clean environment outside my code so I can share more here.

fernandobrito commented 2 years ago

It is reproducible by trying to run the chart_format.py example file.

$ git clone https://github.com/Chavithra/degiro-connector.git
$ cd degiro-connector
$ pip install .
$ pip list | grep pandas
pandas             1.3.4
$ python examples/quotecast/chart_format.py                                                                                                                                                                                                              
Traceback (most recent call last):
  File "examples/quotecast/chart_format.py", line 58, in <module>
    chart0_df = ChartHelper.serie_to_df(serie=chart.series[0])
  File "/Users/brito/personal/degiro-connector/venv/lib/python3.8/site-packages/degiro_connector/quotecast/actions/action_get_chart.py", line 197, in serie_to_df
    return pd.DataFrame(serie.data, columns=columns)
  File "/Users/brito/personal/degiro-connector/venv/lib/python3.8/site-packages/pandas/core/frame.py", line 730, in __init__
    raise ValueError("DataFrame constructor not properly called!")
ValueError: DataFrame constructor not properly called!

Downgrading to pip install "pandas~=1.2.0" solves the issue.

Chavithra commented 2 years ago

how about now ?

fernandobrito commented 2 years ago

@Chavithra great, now it works, thanks!

I was also taking a look myself and I came up with

return pd.DataFrame(cls.message_to_dict(serie.data), columns=columns)

But your solution looks more elegant.

I also ended up writing a unit test for it on my fork, and poetry gave me errors claiming that pandas 1.3.x requires python >= 3.7.1, while the current poetry file has 3.7.

If you want to take the unit test, here is my code: https://github.com/Chavithra/degiro-connector/compare/main...fernandobrito:fix-pandas-compatibility?expand=1

Chavithra commented 2 years ago

Is it possible to create a Pull Request with your modifications ?

Will take a look at it as soon as possible.