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

loosen requirements #76

Conversation

dhpollack
Copy link
Contributor

The requirements of this package are unnecessarily strict. Specifically, I am having issues with tqdm. I have a more in-depth explaination in the issue that I create #75. There are also a few optimizations to your setup.py file. I notice that the requirements.txt file is not used, which could cause a mismatch when doing pip install -r requirements.txt and pip install .

@KennethEnevoldsen
Copy link
Collaborator

Hei @dhpollack sorry I missed this. I have debated the use of requirements.txt and setup being split up. I notice that other libraries keep these split up indicating that requirements also include packages for running the test suite. We, however, haven't done this so far so will change this according to your changes.

@dhpollack
Copy link
Contributor Author

Hei @dhpollack sorry I missed this. I have debated the use of requirements.txt and setup being split up. I notice that other libraries keep these split up indicating that requirements also include packages for running the test suite. We, however, haven't done this so far so will change this according to your changes.

I actually ran into an error when running this and just used your version. But my issue was with importing __version__ instead of the way you are doing it. The issue was that loading the version also loaded __init__.py which loads a bunch of modules before they are installed. The simplest solution would be to directly load those modules instead of putting them in the __init__.py but that does change the user-friendliness of the library a little bit.

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@KennethEnevoldsen KennethEnevoldsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed indentation

setup.py Outdated Show resolved Hide resolved
@KennethEnevoldsen
Copy link
Collaborator

Sorry for the late response on this @dhpollack, seems like there has been an update in one of the workflow packages (nothing wrong with DaCy). I have just updated the workflow scripts in #80 will merge this as soon as that one is done. Enjoy the holidays.

@KennethEnevoldsen
Copy link
Collaborator

@dhpollack I have merged these changes in #82 into the dev branch along with a few other PRs related to GitHub workflow to merge a conflict before committing it to the main branch.

Thanks again for the addition sorry for the late results. Wanted to do an overhaul of the Github actions which ended up taking longer than expected.

Enjoy the holidays 🎄

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

Successfully merging this pull request may close these issues.

None yet

2 participants