Skip to content
This repository has been archived by the owner on Oct 7, 2023. It is now read-only.

Added the ability to retrieve any list of properties #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

billbliss
Copy link

When using this module, I found that unless I wanted just the definition, I had to retrieve all the data for a word.

This PR allows any set of properties to be retrieved, for example just the lemma property or any subset of properties.

I added a properties parameter and changed the semantics of all_data slightly - all_data must be False for the properties parameter to work.

It's backwards compatible: the behavior for just returning the definition is now a default value for properties.

Updated unit tests and documentation. Also changed the .find methods that used text=True to string=True to fix a method deprecation warning.

Had left the original code as a comment and forgot to delete it.
@billbliss
Copy link
Author

I found a minor bug where I forgot to include the format values for the new InvalidPropertyError exception. Can do another PR or revoke this one and resubmit. Missed it because exception messages don't show up when running unit tests of course.

@billbliss
Copy link
Author

billbliss commented Jul 12, 2022

I looked at the code coverage (codecov/patch) checks and the gaps look pretty harmless and appear to have been there before.

@sphoneix22
Copy link
Owner

First of all, thank you for the PR. I'm sorry for the long wait but I've hoped to be able to test the library and merge but I will probably not be able to do it again for a month. I'll try to do it as soon as I can.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants