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

flexible config.py #146

Merged
merged 10 commits into from
Mar 11, 2016
Merged

flexible config.py #146

merged 10 commits into from
Mar 11, 2016

Conversation

juliambrosman
Copy link

More flexibility in config.py for designation of config file.


# First config file found wins
config_file = conf_file[0]
print(config_file)

Choose a reason for hiding this comment

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

What is the purpose of this print ?

@Galithil
Copy link

I understand the idea behind the change, and since it does not break backwards compatibility, I don't have any strong arguments against this PR.
However, I would like you to update the docstring in the beginning, as I believe that
you now must load genologics.config and then run: is not correct, as you don't have to to this, this is an alternate behaviour.

@juliambrosman
Copy link
Author

Yes, I see what you mean. Statement in the docstring was from a previous version that wasn't backwards compatible; should've cleaned it up a bit more before the pull request.

See changes.


Alternate Usage:
from genologics import config
BASEURI, USERNAME, PASSWORD, VERSION, MAIN_LOG = load_config(specified_config = <path to config file>)

Choose a reason for hiding this comment

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

Don't you mean
BASEURI, USERNAME, PASSWORD, VERSION, MAIN_LOG = config.load_config(specified_config = <path to config file>) ?
By using from genologics import config , you only get config in the namespace, not all the declared functions.

@juliambrosman
Copy link
Author

yes, you're right.

@Galithil
Copy link

Okay, this looks good.

Next question is, do you want this to be integrated in the pip package ? If so, we need to update the version.

@btupper
Copy link

btupper commented Mar 11, 2016

Hi,

So, the alternative is that it can wait until they do their own regular integration into pip? Otherwise, you have to install using setup.py? This might be beyond my feeble python skills.

Cheers,
Ben

On Mar 11, 2016, at 10:12 AM, Denis Moreno notifications@github.com wrote:

Okay, this looks good.

Next question is, do you want this to be integrated in the pip package ? If so, we need to update the version.


Reply to this email directly or view it on GitHub #146 (comment).

Ben Tupper
Bigelow Laboratory for Ocean Sciences
60 Bigelow Drive, P.O. Box 380
East Boothbay, Maine 04544
http://www.bigelow.org

@juliambrosman
Copy link
Author

So yes, if it's possible to integrate it into the pip package, that'd be most useful for us.

Thanks!

@Galithil
Copy link

Sure thing.
If in order for that to happen, I'm going to need you to update the file genologics/version.py with the next version number, which should be 0.2.7.

Once this is done, I'll merge and forward the update to pip.

@Galithil
Copy link

Perfect.

Galithil added a commit that referenced this pull request Mar 11, 2016
@Galithil Galithil merged commit bc94ba9 into SciLifeLab:master Mar 11, 2016
@juliambrosman juliambrosman deleted the julia_dev branch March 11, 2016 15:55
@Galithil
Copy link

https://pypi.python.org/pypi/genologics has been updated.

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.

None yet

4 participants