Typing stuff #446
Typing stuff #446
Conversation
|
The point of type-checking all of this is to type-check things, and it feels silly to have to add I will attempt to do some typing for the data. It should be possible, using TypedDict. TypedDict can (in 3.9) have optional and required fields (which are created by basically inheriting). We'll see if I can make it work out, or if I can just get something to work partially. |
|
The proper way to enforce data structure is using pydantic, it would take more efforts than create types. But define typeddict is almost like define pydantic models. And the first two commits are merge commits... |
|
You could try |
|
wtf, those merges don't make sense! This isn't about data structure, this is about type-checking and commenting the code. |
|
IMO add typeddict don't have much benefits compares to pydantic models. Good luck wrestle with mypy... I should say this in another way, I anticipate define typeddict types for JSON data is as difficult as using pydantic models. Maybe even harder for passing mypy checks. |
kristian-clausal
force-pushed
the
typing
branch
from
4a05fef to
003aab5
Compare
|
I couldn't figure out a way to get rid of those merges except merge master to origin/master, push that and then rebase the branch to that (which fixed it). This is why I've been avoiding rebase, it is such a mess. Merges may look messy, but they're clear and simple, while rebases, every time I've touched them, mess something up. Not a fan. |
kristian-clausal
force-pushed
the
typing
branch
from
003aab5 to
8b3fbbb
Compare
|
That's not git rebase's fault. I suspect your "typing" branch is not created correctly. And this pr doesn't even need rebase... |
|
If I can't use a tool properly, then I shouldn't be using that tool! If it makes you any happier, I can just switch back to using merge. |
|
I'm trying to encourage you to try the rebase feature to avoid the situation like the last pr, I'm not even block that pr... Maybe I didn't make my self clear, I just suggest you to rebase to update an outdated pr branch, you could use merge if you don't like rebase. |
kristian-clausal
force-pushed
the
typing
branch
from
8b3fbbb to
fdfbd38
Compare
kristian-clausal
force-pushed
the
typing
branch
2 times, most recently
from
662225b to
6878a0d
Compare
This code used ns_title_prefix_tuple to get a list of
aliases for 'Thesaurus', but then use a constant w[10:]
to remove "Thesaurus" only from the start of `w`.
This might never have actually triggered, if there
are now aliases for "Thesaurus", though!
The more correct way to fix this is to iterate over
the return from ns_title_prefix_tuple, but a less
correct (but probably also correct) would be to just
(revert back to?) just checking w.startswith("Thesaurus")...
Is there anything wrong than seeing a piece of code, thinking to yourself "that's not right, that can't work?", searching for a keyword in there to figure out what the original commit was that introduced this code, and then seeing your own name on that commit? Yeah, this piece of code of has a small error that basically invalidated the whole purpose of the changes made. Happily, the error was to let through everything, which is still *ok*, but the desired result was the filter certain things. The error was: I created a function to remove "pos" fields from "sounds" data when adding that data to a list. The first part of the block was about filtering out "sounds" data that didn't match the word or the forms of the word we were being processed, and the *second* part of the block was about filtering for wrong "pos" data and then removing the "pos" sections.. But I used the pos-removing function in the first part of the block, which meant there were no pos-sections to compare in the second. Because there was not pos-data, all the sounds were let through.
Currently these are TypedDicts with `total=False`, but it is possible in Python 3.9 to have TypedDicts with mixed required and optional fields by using inheritance; one parent TypedDict has the required fields (`total=True`), the other doesn't, and the child TypedDict class has mixed requirements. This can be done later.
Sorry if it's messy, had to do some temp commits and I don't want to touch this anymore after amending this, just in case.
This code has been unreachable for a couple of years, with an XXX comment about removing it; it's pretty hard to figure out if the changes Tatu was meaning to make actually were made, but seeing as how things have been running smoothly even when parse_linkage_template has been unreachable all this time, I opted to just remove the unused code. I've also commented out some data used by parse_linkage_template in linkage_template_mappings (a list of lists that should have been a dict?), and moved the linkage template names into a new template_linkages set to be used in the one place that still had a reference to it.
kristian-clausal
force-pushed
the
typing
branch
from
6878a0d to
3e13e0e
Compare
|
This is getting a bit too long, and most of the changes do not actually affect any behavior so I'll be merging this and will make a new branch in the future. |
Typing stuff, part two, this time so it's possible to rebase (memo to self: don't merge to update, rebase this branch as you go).
I already found a bug. Is there anything worse than looking at some code, seeing that it's wrong, and then finding out it was your own commit that did it?