Skip to content

update Dockerfile.DS to solve series type convertion bugs in kniren/gota #238

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

Closed
wants to merge 1 commit into from

Conversation

DaruiShi
Copy link

As shown in Issue #201 , the bug is in the docker container for data science, when we try to convert some columns' data types, it shows reflect.Value.Convert: value of type reflect.Value cannot be converted to type series.Type.

We identified that the bug is from the go package kniren/gota. But the package go-gota/gota can work well. So I modified the Dockerfile.DS to replace kniren/gota with go-gota/gota.

Also, I updated the Alpine image version and some pre-installed packages and software which solves the cyclic imports error in issue #233 .

@cosmos72
Copy link
Member

Someone with more Docker expertise than me can please review this?
Maybe @SpencerPark or @sbinet ?

Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

with my apologies for the belated review.

@@ -1,4 +1,4 @@
FROM alpine:3.5
FROM alpine:3.14
Copy link
Member

Choose a reason for hiding this comment

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

perhaps bump to 3.15 ?

&& go get -insecure gonum.org/v1/plot/... \
&& go get -insecure gonum.org/v1/gonum/... \
&& go get github.com/kniren/gota/... \
&& export GOINSECURE=gonum.org/v1/plot/...,gonum.org/v1/gonum/...,go-hep.org/x/hep/csvutil/...,go-hep.org/x/hep/fit,go-hep.org/x/hep/hbook \
Copy link
Member

Choose a reason for hiding this comment

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

is GOINSECURE still needed?

Comment on lines 38 to 39
### fix pyzmq to v16.0.2 as that is what is distributed with py3-zmq
### pin down the tornado and ipykernel to compatible versions
Copy link
Member

Choose a reason for hiding this comment

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

perhaps remove these historical comments?

Comment on lines +12 to +13
--repository http://dl-4.alpinelinux.org/alpine/v3.14/community \
--repository http://dl-4.alpinelinux.org/alpine/v3.14/main \
Copy link
Member

Choose a reason for hiding this comment

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

perhaps bump to 3.15 as well?

python3-dev \
mesa-dev \
&& pip3 install --upgrade pip==21.3.1 \
&& cp /usr/bin/python3.9 /usr/bin/python \
## install Go
&& apk --update-cache --allow-untrusted \
--repository http://dl-4.alpinelinux.org/alpine/edge/community \
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed anymore (with >=3.14).
ditto for the --allow-untrusted argument above.

@sbinet sbinet mentioned this pull request Jan 11, 2022
@sbinet
Copy link
Member

sbinet commented Jan 19, 2022

ping?

@sbinet
Copy link
Member

sbinet commented Jan 25, 2022

superceded by #242

@sbinet sbinet closed this Jan 25, 2022
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.

3 participants