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

Updating install scripts for Ubuntu 18.04 - WIP #578

Closed
wants to merge 5 commits into from

Conversation

jamessynge
Copy link
Contributor

Includes updating some dependencies/requirements.

This doesn't fully work yet. astroscrappy build
during install fails. And this is only tested as
by running it on my laptop; I've not yet been able
to run pytest on POCS.

Includes updating some dependencies/requirements.

This doesn't fully work yet. astroscrappy build
during install fails. And this is only tested as
by running it on my laptop; I've not yet been able
to run pytest on POCS.
@wtgee
Copy link
Member

wtgee commented Sep 4, 2018

FYI astroscrappy is broken for everyone and has been for a while. Keeps giving the annoying numpy warnings too.

I'm not sure we are using it anywhere but I think it pulls in with ccdproc, which is annoying.

@codecov
Copy link

codecov bot commented Sep 4, 2018

Codecov Report

Merging #578 into develop will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #578      +/-   ##
===========================================
+ Coverage    70.42%   70.49%   +0.07%     
===========================================
  Files           64       64              
  Lines         5522     5522              
  Branches       769      769              
===========================================
+ Hits          3889     3893       +4     
+ Misses        1425     1422       -3     
+ Partials       208      207       -1
Impacted Files Coverage Δ
pocs/serial_handlers/protocol_arduinosimulator.py 76.92% <0%> (+1.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd792ef...61c341c. Read the comment docs.

@AnthonyHorton
Copy link
Collaborator

I ran into problems with astroscrappy and healpy a little while back and the root cause seemed to be an incompatibility between Cython and Python 3.7, which conda now installs by default. Reverting to Python 3.6 was a successful workaround for me.

This will allow us (eventually) to have our own
channel which provides most packages that we need.

Updated the Google Cloud packages to the currently
supported versions.

Note that Python 3.6 is used because astropy currently
requires it (i.e. can't get go to 3.7).
requirements.txt Outdated
# known repositories that do offer them, but that is
# more of a security risk.

# Python wrapper for ffmpeg.
ffmpy
Copy link
Member

Choose a reason for hiding this comment

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

We could probably lose the option. It is just a thin wrapper around ffmpeg. Since we don't have many varying options it would be trivial to write our own script.

pandas
astroplan
astropy >= 3.0.0
ccdproc
Copy link
Member

Choose a reason for hiding this comment

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

Even though conda fixes the problems with this (via astroscrappy), we are not actually using it anywhere in the repo. Might as well just remove.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove ccdproc?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. It will come through in PIAA probably anyway but might as well get rid of it here since it's not an actual dependency.

requirements.txt Outdated
# https://github.com/ipython/ipython/issues/8249
# https://github.com/jupyter/help/issues/324
# Mock object support for Python testing.
mocket
Copy link
Member

Choose a reason for hiding this comment

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

Could potentially work around this. Since we have requests elsewhere now it might be cleaner to switch TheSkyX calls to requests and then use some of the mock tools provided for it, which are available.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to keep changes in this PR to a minimum (i.e. it will be large enough as is). I'd like to focus on just the switch to conda and 18.04. In fact, maybe I already shoved to much into this, and should extract the 18.04 (Mongodb) changes. Sigh. Lots of slow tests required.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, don't want to do it for this PR, mostly just commenting on what I think we could eventually get rid of.

requirements.txt Outdated

tornado == 4.5.3
# State machine library for Python.
transitions >= 0.4.0
Copy link
Member

Choose a reason for hiding this comment

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

And technically I'm still a maintainer on this project though I haven't done anything for years. 😕 I can find out if there is any plan.

pyserial >= 3.1.1
pytest >= 3.4.0
pandas
python
Copy link
Member

Choose a reason for hiding this comment

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

Does this actually install the python version from here and if so should we specify a version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifying a version is problematic. If you say ==3.7, then the constraints are unsolvable.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha.

Copy link
Member

@wtgee wtgee left a comment

Choose a reason for hiding this comment

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

Sorry, thought I approved this earlier.

jamessynge added a commit to jamessynge/POCS that referenced this pull request Sep 8, 2018
jamessynge added a commit that referenced this pull request Sep 8, 2018
# known repositories that do offer them, but that is
# more of a security risk.

# Thin Python wrapper for ffmpeg. We *could* replace with our own wrapper.
Copy link
Member

Choose a reason for hiding this comment

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

#591 replaces this.

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