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

refactor: remove seaborn as a dependency #145

Merged
merged 1 commit into from
Apr 17, 2020
Merged

Conversation

danielolsen
Copy link
Contributor

@danielolsen danielolsen commented Apr 16, 2020

Purpose

Avoid requiring us to load plotting libraries (seaborn and matplotlib) when all we really need is a string that specifies a color for matplotlib in PostREISE.
Closes #87.

What is the code doing

Specifying our xkcd colors in an equivalent way, without needing to import seaborn to do it.

Removing seaborn from our requirements list.

Time to review

Extremely quick, it's less than 15 lines of changes and most of them are doing the same thing.

@rouille
Copy link
Collaborator

rouille commented Apr 16, 2020

Also, after rebasing onto develop, I created a new environment and did pip install -r requirements. The pip list command shows that seaborn is not installed:

(PowerSimData) [~/REM/PowerSimData] (remove_seaborn) brdo$ pip list
Package            Version
------------------ -------
asn1crypto         1.3.0  
atomicwrites       1.3.0  
attrs              19.3.0 
bcrypt             3.1.7  
cffi               1.14.0 
cryptography       2.4.2  
geographiclib      1.50   
geopy              1.20.0 
idna               2.9    
importlib-metadata 1.6.0  
jsonpickle         1.2    
more-itertools     8.2.0  
numpy              1.16.0 
pandas             0.25.3 
paramiko           2.4.2  
pip                20.0.2 
pluggy             0.13.1 
py                 1.8.1  
pyasn1             0.4.8  
pycparser          2.20   
PyNaCl             1.3.0  
pytest             4.5.0  
python-dateutil    2.8.1  
pytz               2019.3 
scipy              1.2.0  
setuptools         39.0.1 
six                1.14.0 
tqdm               4.29.1 
wcwidth            0.1.9  
zipp               3.1.0  

and, finally, ran the tests. They all pass.

Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the improvement. Please rebase onto develop before merging.

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.

Remove Seaborn as a dependency
3 participants