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

Merge or update calender widget #42

Open
RedFantom opened this issue Nov 30, 2019 · 7 comments
Open

Merge or update calender widget #42

RedFantom opened this issue Nov 30, 2019 · 7 comments

Comments

@RedFantom
Copy link
Member

Right now there are four calender widgets for Tkinter that I know of which all contain a set of features which might be interesting to have.

ttkwidgets version
A simple date picker, really. Built using a Canvas, which results in a pretty compact design. Not many options. Lacks a callback for when a new date is selected.

tkcalendar
A separate package containing an excellent, compactly styles, calendar widget with a huge amount of options, maintained by @j4321 .

tkinterpp version
A Calendar using buttons instead of a Canvas. Uses the Tk widget set currently and a tk.Variable-like variable for storing the date.

gsf-parser version
For the GSF Parser I have developed a widget based on the widgets contained in this package. It acts also as a heatmap. It may be found here.

Which of these widgets should be in the package is the big question here. tkcalendar could be included in an of itself, but I am not sure whether that would be beneficial to that project. Could you give your opinion on this, @j4321?

All in all, I think it might actually be best to remove the Calendar widget from ttkwidgets altogether and just point to tkcalendar. It is by far the best widget out there for this purpose that I know of and building something that does all those things in parallel seems like a waste of time and resources. The only thing I can think off that could be added to tkcalendar is offering multiple colour schemes in the widget itself.

@RedFantom RedFantom added this to To do in Merging tkinterpp widgets via automation Nov 30, 2019
@sbordeyne
Copy link
Member

Well, having a single package full of widgets is always interesting.

Even if installing packages is easy in python, it's still good to not have to "advertise" other packages, and have everything bundled into one (otherwise I'd be working on tkinterpp instead of trying to merge into ttkwidgets). Installing a whole package just for one widget seems wasteful.

The other option is to create one package per widget, which makes the whole installation way more modular, but a pain in the ass to set up.

@sbordeyne
Copy link
Member

The best thing about my implementation, is the variable flow which is very similar to tkinter's StringVar and IntVar for other widgets, which makes it easy to use when you're used to writing code for tkinter.

- Date (DateVar) to be used in Calendar widget
- Path (PathVar) to be used in SelectFileEntry, SelectFolderEntry
- Keybind (KeybindVar) to be used in KeybindingEntry
- Color (ColorVar) to be used in color selector stuff
- NodeVar to be used in NodeView widget

These are the variables I'd like to implement the flow for in ttkwidgets' widgets. I know it goes a bit out of the scope of this issue

@j4321
Copy link
Member

j4321 commented Dec 2, 2019

I have thought a bit about whether we should merge tkcalendar into ttkwidgets. One of the issue is that people use tkcalendar as a separate package and totally merging it into ttkwidgets would break dependencies in other people's projects. And given the github stats, this module is already used by a number of people.

So I think that we could either choose one of the other calendars for ttkwidgets and make it a basic but functional widget, well integrated in the package or we could delete the calendar widget from ttkwidgets and I would transfer tkcalendar to the team as a separate package.

@RedFantom
Copy link
Member Author

Personally, I do not see any added benefit of having another complex Calendar widget, so I would indeed agree that either the current Calendar widget should be removed, or at most it should be extended only a little.

If the Calendar widget is not deleted but merely modified, then it indeed should integrate well into the package and the topic of this issue shifts to what the proper interface would then be.

@Dogeek's implementation with DateVar similar to the way the IntVar and other variables work are indeed very familiar to those familiar with Tkinter, but they are also based on a system rooted in the Tk toolkit itself and not necessarily Pythonic. Using properties is a little more Pythonic. Using both leads to ambiguity in the interface and possibly data duplication.

I'll think about what I find to be the best option, but regardless, all widgets in the package should work in a similar way.

@sbordeyne
Copy link
Member

sbordeyne commented Dec 2, 2019

@RedFantom I know having these IntVar, StringVar or even DateVar is not really pythonic, but truth be told, neither are properties in that case.

With tkinter and other toolkits, you kind of want to have variables separated from the widgets, otherwise it becomes quite the headache to follow common design patterns, such as the popular MVC pattern. Properties on the widget kind of suck in that sense in that we're forcing people to mix their views/widgets with their data. With tkinter/our variables, you can have a single model class with every variable defined inside, and process the data from there. On big projects, it makes a huge difference to be able to see the data flows clearly.

As for the Calendar widget implementation, I'm not opposed to scrapping the Calendars we made to the profit of @j4321 's tkcalendar module. I wonder if we could not just import tkcalendar in ttkwidgets, and just provide tkcalendar's Calendar widget directly. Something like :

# ttkwidgets/ttkwidgets/__init__.py
from tkcalendar import Calendar, DateEntry
# ttkwidgets/setup.py
...
    install_requires=["pillow", "tkcalendar"]

This would be the best of both worlds, you can just install ttkwidgets, and have access to a great Calendar widget, but tkcalendar still exists, so it doesn't break dependencies.

@RedFantom
Copy link
Member Author

@Dogeek I agree with you that separating data from widgets is a good design practice. I have not always done so in the past and it is easier to just keep reference of a single object in some cases, but as soon as you start passing the data around, using a Variable is indeed easier. Variables are not flawless, however. For example, their behaviour when it comes to threads is non-obvious. This might be worth a consideration.

While I have given the option of including tkcalendar as a requirement some thought, I do not think it is the best course of action. The benefit from including it in that way is small, yet it removes the possibility for the user to exclude the Calendar widget if it is not needed, adding another package that might fail or cause dependency conflicts in return. This could possibly be solved with a except ImportError, but that would make the behaviour of the library non-obvious. Therefore, I think it would be best just to remove all Calendar widgets or keep just a basic one. I do not have a strong preference either way which option we choose: Keeping a single basic widget will not be much effort, while scrapping them completely will not let much effort go to waste either. I am lightly inclined to scrap the widgets, though.

@sbordeyne
Copy link
Member

The only problem I have with not having the Calendar widget in ttkwidgets is that it's a module that is not too popular in the first place (I mean, neither is ttkwidgets or tkinterpp to be honest), so it's kind of hard to find, hence having just one module with everything included is quite desirable.

Ultimately, it's your choice. I like my implementation of the Calendar widget (given that I use buttons directly, and use a tkinter-like Variable to hold the date), which is really easy to style, and use (in my opinion). If we were to keep a Calendar, I'd move that my implementation is slightly better than the one in ttkwidgets, and that we should use it, albeit replacing the tk.Button widgets with ttk.Button ones.

If we scrap it then fine, but tkcalendar should be included in the TkinterEP team repos.

I understand that Variables have their flaws, but my own implementations are pure python, and wouldn't cause Thread lock issues, unlike the default tkinter ones. They are really handy to separate Data from Actions after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

3 participants