Hold-Krykke / PythonExam

4. Semester Python Eksamens Projekt
1 stars 1 forks source link

args, kwargs in presentation #15

Open Castau opened 4 years ago

Castau commented 4 years ago

Moved this to it's own separate isuue, instead of #12

See df.plot documentation, and notice how you can pass kwargs to matplotlib:
https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.plot.html

So my guess is that it can be done like this: (pseudo)

def line_plot(df, title, save=None, **kwargs):
    # <...>
    df.plot(kind="line", title=title, kwargs=kwargs)
    # <...>

More on stackoverflow:
https://stackoverflow.com/questions/29504573/panda-dataframe-passing-in-parameters-for-plotting

Another stackoverflow example:

def valid_mpl(**kwargs):
    fig, ax = plt.subplots()

    fake_kwargs = ['my_fake_kwarg']
    plot_kwargs = {k: v for k, v in kwargs.items() if k not in fake_kwargs}

    ax.plot([1,2,3], [-3, -2, -1], **plot_kwargs)
    return fig, ax
MrHardcode commented 4 years ago

I think this issue is outdated now. Settings on the different plots have been fine tuned here #13 kwargs are not needed anymore. We would have to change code a lot of places in both flask_service.py and presentation.py to make the changes proposed in this issue.

Runi-VN commented 4 years ago

I don't agree with kwargs are not needed anymore. Don't get me the wrong, the plots look good but they have so much more potential. Shifting that potential over on the user, makes it easier for both them and us. Right now there are no plot customization options, except for filtering, which isn't really what kwargs are about.

I don't have an overview over how many changes would have to be made, so I can't comment on that. My initial thoughts are, that the changes aren't that big. Take argument in -> Handle and pass to presentation.py -> pass to df.plot (which itself passes it to matplotlib).

Then again, I can maybe see this as a nice to have, rather than need to have.

I think this is a nice way to show deeper knowledge of how Python modules work. If we decide against implementing it, I suggest everyone to make a note of how this could have been done.

MrHardcode commented 4 years ago

You're mentioning several valid points that I didn't think about. I just thought that we were going to leave the presentation as it is since we're already transitioning to working on the other project. I also don't know much about plotting which is why I thought we were done customizing presentation and that we didn't need kwargs anymore. I think I need some examples of the possibilities that kwargs creates to understand it's importance. I still think it's a nice to have feature though since the plots are alright as they are now. We need @Castau 's and @MalteMagnussen 's opinions.

Runi-VN commented 4 years ago

Yeah, I'm leaning towards nice to have as well. Sadly. 😄

For example, Camilla talked about grids earlier. We could allow users to add grids themselves by passing grid-keywords directly into the main program, which we passed down to df.plot().

Do remember that this is all speculation. I haven't tried any of this, and I am not sure if we can target args directly to plots, axes, or only to the pyplot itself.

For an example for bar plots alone, these are the kwargs available:
https://matplotlib.org/3.2.1/api/_as_gen/matplotlib.pyplot.bar.html

Castau commented 4 years ago

I agree with @Runi-VN, I think it would show a deeper understanding. But As @HrBjarup said, we unfortunately have to start working on the next project, which is twice as big as this one. I suggest we leave it for now and keep it open as an issue. If anybody finds themselves with nothing to do, in the next couple of weeks, they kan look in to that issue.