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

Switch from conda to venv for development env #1297

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

Conversation

bloomsa
Copy link
Contributor

@bloomsa bloomsa commented May 6, 2023

addresses #1294

Upon doing this, the "happy path" is simpler but overall it feels a little more frail / platform dependent. I'm curious to hear thoughts on that tradeoff or any suggestions people have seen from other repos

@bloomsa bloomsa marked this pull request as draft May 6, 2023 01:27
@bloomsa bloomsa marked this pull request as ready for review May 8, 2023 22:50
@lresende
Copy link
Member

lresende commented May 9, 2023

Could you elaborate on the why?

@bloomsa
Copy link
Contributor Author

bloomsa commented May 9, 2023

@lresende beyond what is in issue #1294?

@lresende
Copy link
Member

lresende commented May 9, 2023

is there a way to make it a choice? instead of one or another?

@kevin-bates
Copy link
Member

Or remove this topic altogether. To my earlier point, the environment is up to the dev, so should we even bother with providing these trivial helper methods at all? We can document suggestions, but do we need the make target helpers?

@bloomsa
Copy link
Contributor Author

bloomsa commented May 10, 2023

We can document suggestions, but do we need the make target helpers?

I like this idea. I can update the docs to include suggestions for both venv and conda, but pull out the make targets altogether. On one hand, it feels kinda wrong to pull out something that already works, but my thinking behind it is that having the targets there makes it seem like conda is the only way to build the local dev environment when that isn't really the case.

Are you both okay with that solution @lresende @kevin-bates

@kevin-bates
Copy link
Member

Hi @bloomsa - I apologize for the delay. I'm fine with documenting suggestions. I just don't really see the utility of having helper targets at this point.

@lresende - do you have a strong objection?

@lresende
Copy link
Member

@kevin-bates Well, it looks like we have been there:
#1079

And than we came back:
#1108

Do you remember why we had to come back?

@kevin-bates
Copy link
Member

It appears we have come full circle. 😄 @bloomsa contributed #1107 to introduce only the helper targets, but then we hit a bit of a snafu due to other changes that #1108 fixed.

Given this, I think we can proceed with removing the helper targets and documenting suggestions for setting up conda/mamba and venv environments - as discussed above.

@lresende, @bloomsa - does this sound like a plan?

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

Successfully merging this pull request may close these issues.

None yet

3 participants