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

Feature/#32 dotenv #38

Merged
merged 10 commits into from
Mar 27, 2020
Merged

Feature/#32 dotenv #38

merged 10 commits into from
Mar 27, 2020

Conversation

VikashKothary
Copy link
Contributor

@VikashKothary VikashKothary commented Mar 17, 2020

Depends on #35. Resolves #32

If merged, we can then use the environment variables when using the Docker image.

@cjekel cjekel added this to the 1.0.0 milestone Mar 18, 2020
@cjekel
Copy link
Owner

cjekel commented Mar 18, 2020

  • release version 1.0.0 with this change
  • update documentation for this
  • add note in change log
  • test this change on Windows 10 later today

Copy link
Owner

@cjekel cjekel left a comment

Choose a reason for hiding this comment

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

Need to change the error txt in def command_line_run():

    if defaults['facebook_token'] is None and defaults['XAuthToken'] is None:
        raise('ERROR: No facebook token nor XAuth token in config.txt. '
              'You must supply a facebook token in order to use tindetheus!')

@cjekel
Copy link
Owner

cjekel commented Mar 19, 2020

Is there a Windows equivalent to a .env?

Windows users can either set the environment variables the usual way, or create a local batch script to set them (which they run prior to executing tindetheus). You could do A/B testing with the same model using a.bat and b.bat!

The env.bat file could look like

@ECHO OFF

rem Tindetheus
set TINDETHEUS_MODEL_DIR=20170512-110547
set TIDETHEUS_IMAGE_BATCH=1000
set TINDETHEUS_DISTANCE=5
set TINDETHEUS_LIKES=100
set TINDETHEUS_RETRIES=20

rem Tinder
set TINDER_AUTH_TOKEN=TODO

rem Facebook
set FACEBOOK_AUTH_TOKEN=TODO

This is a great change! and it gets rid of some of the worst code.

@VikashKothary
Copy link
Contributor Author

This is interesting. The above sounds like a suitable solution. It's probably the best for most users because they can set it globally then don't need to worry about it when running the program.

Another solutions could be to use the https://pypi.org/project/python-dotenv/ library. I planned on using it when creating some example notebooks for #31 and it should work on all OSs. It works by reading the same .env as the Makefile.

@VikashKothary
Copy link
Contributor Author

Please test the new Windows compatibility. Running the project like normal (without the Makefilie) should automatically pick up the .env variables. There is also the Windows script like you've suggested.

Regards #38 (comment), I've updated the documentation. Please have a look and let me know if there's anything I'm missed.

I imagine the version and changelog changes should be make on a separate branch as it is not related to the dotenv feature?

@cjekel
Copy link
Owner

cjekel commented Mar 26, 2020

*EDITED March 25 2020 19:46 pacific time: Nevermind, the .env didn't work when I moved the from tindetheus import config. Do you know why the script (tindetheus.exe) is only picking up the .env from the installation folder and not the local folder? I imagine making a function to run config.py could help resolve this issue, but I'm not sure. Will play around more when I get free time. I removed my suggestion about placing from tindetheus import config to within the main loop.


  1. So I noticed that the .env would only get picked up (on Windows) from the installation directory (site-packages/tindetheus/.env). I think this has something to do with how I make tindetheus script.

  2. Thanks for catching my typos. There's another MIM which could be MITM in getting_started.md

  3. There is still a note in the help section of tindetheus.py which needs similar changes to readme.

'''
Settings are stored in your config.txt file. A typically config.txt will
contain the following:
facebook_token = XXXXXXX  # your facebook token hash
model_dir = 20170512-110547  # the location of your model directory
image_batch = 1000  # number of images to load in a batch during train
# the larger the image_batch size, the faster the training process, at the
# cost of additional memory. A 4GB machine may struggle with 1000 images.
distance = 5  # Set the starting distance in miles
likes = 100  # set the number of likes you want to use
# note that free Tinder users only get 100 likes in 24 hours
\n
Optional arguments will overide config.txt settings.
'''

could be

'''
You can now store all default optional parameters in your environment variables! This means you can set your starting distance, number of likes, and image_batch size without manually specifying the options each time. This is an example .env file:
FACEBOOK_AUTH_TOKEN="TODO" # your facebook token hash
# alternatively you can use the XAuthToken
# TINDER_AUTH_TOKEN="TODO"
TINDETHEUS_MODEL_DIR="/models/20170512-110547"  # the location of your facenet model directory
# see https://github.com/davidsandberg/facenet#pre-trained-models for other
# pretrained facenet models
TINDETHEUS_IMAGE_BATCH=1000  # number of images to load in a batch during train
#  the larger the image_batch size, the faster the training process, at the
#  cost of additional memory. A 4GB machine may struggle with 1000 images.
TINDETHEUS_DISTANCE=5  # Set the starting distance in miles
TINDETHEUS_LIKES=100  # set the number of likes you want to use
#  note that free Tinder users only get 100 likes in 24 hours
TINDETHEUS_RETRIES=20
'''
  1. Version and change log I'll do when I make a new release. No need to worry about these. :) Thanks for the very nice additions.

  2. I'm flagging this as a 1.0.0 release because it breaks some backwards compatibility. Is there anything else you plan on adding to accompany this release?

@cjekel
Copy link
Owner

cjekel commented Mar 26, 2020

I should add that the .env works fine on windows when running bits from a jupyter notebook, ipython or python console, but it doesn't work when calling the run script tindetheus.exe.

ie running

python
>>> from tindetheus import config

sets config properly on windows

@cjekel
Copy link
Owner

cjekel commented Mar 27, 2020

I think the version agnostic way to look for a .env file on windows will end up being

import os
from dotenv import load_dotenv
cwd = os.getcwd()
load_dotenv(dotenv_path=os.path.join(cwd, '.env'))

but let me test if that creates linux issues.

@cjekel cjekel merged commit da31630 into cjekel:master Mar 27, 2020
cjekel added a commit that referenced this pull request Mar 27, 2020
from the discussion in #38

This change uses dotenv to search the current working directory for a
.env file. Hopefully this is a good comprimise between linux and
Windows compatibility.

Also this changes adds the MIT license to the top of file
VikashKothary pushed a commit to Vikash-Kothary/tindetheus that referenced this pull request Mar 28, 2020
from the discussion in cjekel#38

This change uses dotenv to search the current working directory for a
.env file. Hopefully this is a good comprimise between linux and
Windows compatibility.

Also this changes adds the MIT license to the top of file
VikashKothary added a commit to Vikash-Kothary/tindetheus that referenced this pull request Mar 28, 2020
from the discussion in cjekel#38

This change uses dotenv to search the current working directory for a
.env file. Hopefully this is a good comprimise between linux and
Windows compatibility.

Also this changes adds the MIT license to the top of file
@VikashKothary VikashKothary mentioned this pull request Mar 28, 2020
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.

Use Environment Variables
2 participants