-
Notifications
You must be signed in to change notification settings - Fork 13
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
Deprecate odm2 #147
Deprecate odm2 #147
Conversation
@ocefpaf Is there a way to ignore prints within code so that travis ci is not complaining or do I have to put |
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. |
Addresses #146 |
We can drop |
@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_odm2 Thanks 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 |
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? |
@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 . |
@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. |
@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? |
What do you think of |
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 |
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. |
All works, got @emiliom blessing 😉 Merging! 😃 |
Overview
This is an alternate to #145