-
Notifications
You must be signed in to change notification settings - Fork 39
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
[DH-140] clean up the datahub image python install #5274
[DH-140] clean up the datahub image python install #5274
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay for deleting a whole bunch of stuff for courses that now have their own hub!
I left one comment about prob140 dependencies.
Ideally, packages should be moved from the pip section to the upper conda package list, if those packages are available through conda. My feeling is that this will improve the performance of the resolver during installation.
Lastly, for dependencies with descriptions referring to very old terms (like Fall 2019), either those dependencies should be removed, or the comments should be updated to reflect that the dependencies are need through specific upcoming semesters. (like if it is known that the course will be taught by the same instructor for the next X semesters)
That being said, even if you agree with any of this, it doesn't all need to happen in this PR.
yeah, these are all great suggestions and i'll address as much as i can in
this PR.
the first thing i'd like to do is insert some timing to compare the overall
decrease in build time vs the original `environment.yml`. sadly, i don't
think i'd be able to split out the conda vs pip section in there to see the
difference when moving things to the conda block.
…On Thu, Dec 14, 2023 at 5:58 PM Ryan Lovett ***@***.***> wrote:
***@***.**** commented on this pull request.
Yay for deleting a whole bunch of stuff for courses that now have their
own hub!
I left one comment about prob140 dependencies.
Ideally, packages should be moved from the pip section to the upper conda
package list, if those packages are available through conda. My feeling is
that this will improve the performance of the resolver during installation.
Lastly, for dependencies with descriptions referring to very old terms
(like Fall 2019), either those dependencies should be removed, or the
comments should be updated to reflect that the dependencies are need
through specific upcoming semesters. (like if it is known that the course
will be taught by the same instructor for the next X semesters)
That being said, even if you agree with any of this, it doesn't all need
to happen in this PR.
------------------------------
In deployments/datahub/images/default/environment.yml
<#5274 (comment)>
:
> - - jassign==0.0.7
- - dsassign==0.0.8
-
- # data102
- - dask==2021.10.0
- - distributed==2021.10.0
- - keras-applications==1.0.8
- - keras-preprocessing==1.1.2
- - keras==2.11.0
- - keras-vis==0.4.1
- - plotly-express==0.4.1
- - cytoolz==0.11.0
- - pyro-ppl==1.7.0
- - lime==0.2.0.1
- - shap==0.39.0
-
# prob140 2021 Spring
We should either delete prob140 dependencies here, or preserve them,
delete the prob140 hub, and have the prob140 instructor use datahub. The
prob140 hub was created in order to isolate GSI access to student materials
-- i.e. when data8 GSIs might be prob140 students, but this is now
addressed by scopes and roles. Deleting the prob140 hub would require
@balajialg <https://github.com/balajialg> / @ericvd-ucb
<https://github.com/ericvd-ucb> to work with its course staff to ease the
migration.
Deleting the dependencies here and preserving prob140 hub (or just doing
nothing at this point) is the easier option, but I don't think its the
right one long term.
—
Reply to this email directly, view it on GitHub
<#5274 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMIHLD6HTBWRBFSMTHOG43YJOVFNAVCNFSM6AAAAABAVWEJL6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOBTGAYDSOJTGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
i ran the following command on a list of pip packages i gleaned from
then i went through, confirmed they were there and moved them out of the pip block: 6d57c86 now we can check the mamba build steps w/timing and compare to how it was before! |
tl;dr: it didn't appear that having conda resolve more packages than pip would have been worth the effort to implement. also, bumping the circleci runner size from 'default' to 'large' didn't appear to make the build go any quicker that before. the final build is running and i'll be merging to staging afterwards. |
DO NOT MERGE (yet)