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

Add method for pip dependencies #25

Merged
merged 8 commits into from
Jun 16, 2021
Merged

Add method for pip dependencies #25

merged 8 commits into from
Jun 16, 2021

Conversation

KevinCawley
Copy link
Contributor

This is an addition to allow pip dependencies to be installed easier, so that a custom install command isn't needed to be made.

Amend pip_dependency to make it a list, and added that functionality to the install command to allow for a list to be processed.
Change return from NotImplementedError to empty list
@esc
Copy link
Member

esc commented May 26, 2021

@KevinCawley looks good, the tests are passing too so that's good and I like the way they are optional! Is there anything that can be done to add testing this feature to either the system or the unit tests of this (texasbbq) package?

@KevinCawley
Copy link
Contributor Author

KevinCawley commented May 26, 2021

I'm not sure how I could add any additional testing to CircleCI. I could import some other tests from the numba_integration_testing and add them temporarily to it in order for those to run if you think that's a good idea.

One thing I did not is that there hasn't been a new release of Texasbbq in almost two years, so when I downloaded this to do some local testing it didn't have functions like git_ls_remote_tags and others, so another release may be a good thing.

@esc
Copy link
Member

esc commented May 26, 2021

I'm not sure how I could add any additional testing to CircleCI. I could import some other tests from the numba_integration_testing and add them temporarily to it in order for those to run if you think that's a good idea.

My suggestion would be to add a fake pip dependency or two to the umap target, and at the end maybe ensure that the package were installed in the target environment. There should be sufficient utility methods to implement this.

One thing I did not is that there hasn't been a new release of Texasbbq in almost two years, so when I downloaded this to do some local testing it didn't have functions like git_ls_remote_tags and others, so another release may be a good thing.

Yes, I only ever made an initial release to secure the package name on PyPi. Since then, the integration testing has been pip installing from the github master, so the lack of current releases never caused a problem.... until now. So, my suggestion would be to get this feature you are working on in and then push out a new release. (Release procedure is manual for now) How does that sound?

@KevinCawley
Copy link
Contributor Author

That sounds good. I can work on the umap later tonight and tomorrow.

@KevinCawley
Copy link
Contributor Author

Looks like I won't be able to make another commit until Sunday, sorry.

@esc
Copy link
Member

esc commented May 31, 2021

@KevinCawley no worries at all sir, no pressure. If you run out of time, just let me know and I'll take over.

@KevinCawley
Copy link
Contributor Author

@esc I have put in a few hours trying to figure out how to test for the packages using the unit_test file and have been unable to. I think I will be passing the torch to you for the testing part unfortunately. Sorry.

@KevinCawley
Copy link
Contributor Author

@esc I realized that I can simply try to import the pip modules in a test function and see if I get an error. I wasn't sure how I would be able to make these tests, I didn't realize it was that simple. I will try to have this done by today. As a question, should I catch the ModuleNotFoundError or just let pytest deal with it?

@esc
Copy link
Member

esc commented Jun 16, 2021

@esc I realized that I can simply try to import the pip modules in a test function and see if I get an error. I wasn't sure how I would be able to make these tests, I didn't realize it was that simple. I will try to have this done by today. As a question, should I catch the ModuleNotFoundError or just let pytest deal with it?

Oh, that'S great news! Not sure, about the error, maybe let pytest just deal with it?!

@KevinCawley
Copy link
Contributor Author

@esc New methodology, tests passed.

@esc
Copy link
Member

esc commented Jun 16, 2021

@KevinCawley awesome, thank you! I'll merge this now. If you want, please do open a tiny PR for the numba-integration-testing repo to upgrade the TardisTests to use this feature. The numba-integration-testing setup on circle-ci always pip installs texasbbq from the git master so it should pick it up fine. That would also be a good cross-check to watch this run "in production".

@esc esc merged commit e181662 into numba:master Jun 16, 2021
@KevinCawley
Copy link
Contributor Author

@esc New release time? 😄

@esc
Copy link
Member

esc commented Jun 16, 2021

@KevinCawley indeed, but I need to delay that by about one week, we are getting ready to tag Numba an llvmlite next week and there are still quite a number of tasks left. We will tag those next Thursday and so I'll have plenty of time 🤞 to tag and release texasbbq on Friday. I just noticed, it has neither a set of "maintainers notes on how to cut a release" nor a changelog (this will be the first meaningful release, besides squatting the project on PyPi). So I'd like to add those and not need to rush it out the door. Thank you in advance for your understanding and patience. 🙏

@KevinCawley
Copy link
Contributor Author

Just checking in on the new tag, but no rush.

@esc
Copy link
Member

esc commented Jul 20, 2021

@KevinCawley still in the release period, so sorry. Soon, I hope!!

@KevinCawley
Copy link
Contributor Author

Just another check-in, more out of curiosity than anything. I can see how the pip dependencies aren't a big change though.

@esc
Copy link
Member

esc commented Sep 23, 2021

Just another check-in, more out of curiosity than anything. I can see how the pip dependencies aren't a big change though.

Oh, yes, thank you for the ping, I had completely forgotten about this 🤷

@esc
Copy link
Member

esc commented Sep 30, 2021

@KevinCawley can you review #27 -- I'll tag and PyPi upload then

@KevinCawley
Copy link
Contributor Author

KevinCawley commented Oct 1, 2021 via email

@esc
Copy link
Member

esc commented Oct 1, 2021

@KevinCawley https://pypi.org/project/texasbbq/0.2.1/ -- had some issues with RST on PyPi so I reduced the amount of metadata on PyPi.

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.

2 participants