Uh oh!
There was an error while loading. Please reload this page.
Deprecate odm2 - #147
Conversation
lsetiawan
commented
Feb 16, 2018
lsetiawan
commented
Feb 16, 2018
@ocefpaf Is there a way to ignore prints within code so that travis ci is not complaining or do I have to put |
emiliom
commented
Feb 16, 2018
I don't follow. But maybe I should just call you to ask you about it ...
Is that what those are?! I'd seen them before somewhere and thought they were plain old comments. |
emiliom
commented
Feb 17, 2018
Addresses #146 |
We can drop |
emiliom
commented
Feb 20, 2018
@lsetiawan I tried to create a local conda env based on your # I downloaded requirements.txt and requirements-dev.txt from your fork (I'd already done a git clone)
conda create --name odm2api_dev python=2.7 --file requirements.txt --file requirements-dev.txt -c odm2
source activate odm2api_dev
pip install git+https://github.com/lsetiawan/ODM2PythonAPI.git@deprecate_odm2Thanks for your help! |
If you already done a git clone and you have installed the dependencies, you can just cd into the repository and do |
emiliom
commented
Feb 26, 2018
I tried this ( When I start Then I tried installing the latest odm2api from conda (odm2 channel) the normal way, just to double check: Everything was fine. The up-to-date ipython and python versions are installed, and no problem importing odm2api. So, I don't know what's going on, but I'm not able to test your branch 😢. Any ideas? |
lsetiawan
commented
Feb 26, 2018
@emiliom You're having trouble because your git clone https://github.com/ODM2/ODM2PythonAPI.git
cd ODM2PythonAPI/
conda create --name odm2api_dev -c conda-forge python=2.7 ipython --file requirements.txt --file requirements-dev.txt source activate odm2api_dev
pip install -e . |
emiliom
commented
Feb 27, 2018
@lsetiawan: Alright! I finally created a local conda dev environment with a good ipython and jupyter, then tested this using an old notebook that examines the LBR SQLite DB. It worked as expected both without the deprecated ODM2 package hierarchy, and with it (which in turn produced the obnoxious deprecation warnings, as planned 😼) I also reviewed the changes to the modules, and everything looks good (nice strategy, BTW!). But given that TravisCI is failing and I don't know enough to figure out why (ie, to see if it's those print statements you referred to earlier), I'll let you merge the PR. |
ocefpaf
commented
Feb 27, 2018
@lsetiawan if you are going to use print as a mechanism to output to stdout you can remove |
@ocefpaf, What do you suggest is a better mechanism to output to stdout? |
lsetiawan
commented
Feb 27, 2018
What do you think of |
ocefpaf
commented
Feb 27, 2018
I would not say better b/c I don't really know the differences and possible issues that arise from using a simple With that said. I am 100% fine to just accept the |
emiliom
commented
Feb 27, 2018
I think we should stick with accepting the Besides, this PR has nothing to do with that issue. Let's move forward with this PR. |
lsetiawan
commented
Feb 27, 2018
All works, got @emiliom blessing 😉 Merging! 😃 |
Overview
This is an alternate to #145