Skip to content

ENH: Adding docstrings to functions in config reader #81

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.

Sign up for GitHub

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

Open
aplymill7 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

ENH: Adding docstrings to functions in config reader #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52d146f
ENH:Adding docstrings to functions in config reader
aplymill7 Sep 18, 2019
f3d5607
STYL: out_params doc strings
aplymill7 Sep 21, 2019
ad7d0d6
STYL: Improving the docstrings.
aplymill7 Sep 22, 2019
e38fbc5
STYL: making docstring changes
aplymill7 Sep 22, 2019
4b452bd
ENH: changing plotter from time -> duration
aplymill7 Sep 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 167 additions & 10 deletions pytentiostat/config_reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,23 @@

def parse_config_file(configlocation=None):
"""
Reads config data from the config file

@sbillinge sbillinge Sep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line should be put back in. First line is a one-line (<80 chars) description of the overall purpose of the function, then a blank line, then as many lines as you want to expand on the explanation, then a blank line, then Parameters underlined and so on.

@aplymill7 aplymill7 Sep 22, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Okay, makes sense!

the config file must be called config.yml.
Reads config data from the config file.
The config file must be called config.yml.

Parameters
----------
configlocation : str
the path on the filesystem to the config file. Optional. If not
configlocation : str, optional
The path on the filesystem to the config file. If not
specified pytentiostat looks in the current directory for the
config files.
config file.

Returns
-------
config_data : dict
the configuration data

The configuration data.
"""

if not configlocation:
configlocation = "./"
try:
Expand All @@ -42,6 +42,26 @@ def parse_config_file(configlocation=None):
sys.exit("Directory containing config file, {}, not found. Exiting...".format(configlocation))

def get_output_params(config_data, override_ts=None):
"""
Returns the output filename and output filepath from the config file data.

Parameters
----------
config_data : dict
The configuration data
override_ts : str, optional
Unique string to append to output filename string. If not specified will add a time stamp
hours, minutes, and seconds the experiment was conducted at as a unique identifier.

Returns
-------
out_name_ts : str
The output filename with unique identifier. The unique identifier is the time in hours, minutes, and seconds
at which the experiment was conducted.
data_out_path : str
The path to the directory to which the file will be written.
"""

data_out_name = config_data["general_parameters"]["data_output_filename"]
data_out_path = config_data["general_parameters"]["data_output_path"]
if data_out_path.lower() == "desktop":
Expand All @@ -56,6 +76,24 @@ def get_output_params(config_data, override_ts=None):


def get_lsv_params(config_data):
"""
Returns the parameters for a linear sweep voltammetry experiment from the config data.

Parameters
----------
config_data : dict
The configuration data

Returns
-------
start_voltage : float
The voltage to start the sweep at in volts.
end_voltage : float
The voltage to end the sweep at in volts.
sweep_rate : float
The rate at which the experiment is performed in millivolts per second.
"""

start_voltage = config_data["linear_sweep_voltammetry"]["start_voltage"]
end_voltage = config_data["linear_sweep_voltammetry"]["end_voltage"]
sweep_rate = config_data["linear_sweep_voltammetry"]["sweep_rate"]
Expand All @@ -64,13 +102,52 @@ def get_lsv_params(config_data):


def get_ca_params(config_data):
"""
Returns the parameters for a chronoamperometry experiment from the config data.

Parameters
----------
config_data : dict
the configuration data

Returns
-------
voltage : float
The voltage which will be held constant over the course of the experiment in volts.
time: float
How long the voltage will be held in seconds.

"""

voltage = config_data["chronoamperometry"]["voltage"]
time = config_data["chronoamperometry"]["time"]

return voltage, time


def get_cv_params(config_data):
"""
Returns the parameters for a cyclic voltammetry experiment from the config data.

Parameters
----------
config_data : dict
the configuration data

Returns
-------
start_voltage : float
The voltage to start the experiment at in volts.
first_turnover : float
The voltage at which the voltage sweep will first change directions in volts.
second_turnover: float
The second voltage at which the voltage sweep will change directions in volts.
sweep_rate : float
The rate at which the experiment is performed in mV/s.
cycle_number: int
How many times to perform this cycle.
"""

start_voltage = config_data["cyclic_voltammetry"]["start_voltage"]
first_turnover = config_data["cyclic_voltammetry"]["first_turnover_voltage"]
second_turnover = config_data["cyclic_voltammetry"]["second_turnover_voltage"]
Expand All @@ -81,29 +158,108 @@ def get_cv_params(config_data):


def get_exp_type(config_data):
"""
Returns the type of experiment from the config data.

Parameters
----------
config_data : dict
The configuration data

Returns
-------
exp_type : str
The type of experiment to be run.
"""

exp_type = config_data["general_parameters"]["experiment_type"]

return exp_type


def get_exp_time(config_data):
def get_exp_duration(config_data):
"""
Returns how long the experiment is to be run for the chronoamperometry case.

@sbillinge sbillinge Sep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why isn't this info returned with the chronoamperometry info? Wouldn't that be better maybe?

@aplymill7 aplymill7 Sep 23, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I just wrote this for plotting module since the only data it needs for the CA experiment is the time (not the voltage). But maybe this one is separate enough that I should just read from the dictionary in plotter and get rid of this function. Check for it could already be made by get CA params.


Parameters
----------
config_data : dict
The configuration data

Returns
-------
exp_time : float
How long the experiment will be run in seconds.
"""

exp_time = config_data["chronoamperometry"]["time"]

return exp_time


def get_rest(config_data):
"""
Returns how long to hold before experiment starts from the config data.

Parameters
----------
config_data : dict
The configuration data

Returns
-------
rest_time : float
How long the starting voltage will be held in seconds.

"""

rest_time = config_data["general_parameters"]["rest_time"]

return rest_time

def get_steps(config_data):

@sbillinge sbillinge Sep 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't quite get why we are having functions that just retrieve dictionary values..... I guess it can make sense if it is part of our user-interface, and we don't expect our users to know python very well. Is that the case?

@aplymill7 aplymill7 Sep 23, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Though I expect users won't know python well, this part won't really be front-end. I envision in the end, user will enter into visual boxes which, when starting experiment, will write a config (which can be loaded if desired to conduct the same experiment later). I just thought compartmentalizing the experimental config reading here might be good so these functions can read in and then go through a series of checks to make sure they are appropriate values (I still don't have these checks in for most of these functions) instead of bundling that with the functions of the other modules. I can just remake it and place the reading and the checks in the other modules where they are used though if that makes more sense.

@sbillinge sbillinge Sep 24, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see. Actually, this is part of the MVC pattern and it would be the controller layer that does the validation, but it would probably be structured differently with a few validation functions that are more abstract (think about validating file paths, emails or passwords), but here we would be validating that floats are within some range or sthg and then return true or false and it would have statements like

if validate_voltage(config_data["ca"]["initial_voltage"], -5., 5.):
   ca_initial voltage = config_data["ca"]["initial_voltage"]

We can certainly do it your way, but we are writing a lot of code and a way that does it robustly with less code is probably preferred.

Maybe we can discuss this a bit more on the call and make a decision. it will also depend on the UC a bit, so spending time hardening this code (docstrings and tests) may not be the best just yet (though I am glad this is now your impulse and wanting the code to validate!). I think it has also been helpful for learning. I am very excited about how you are now thinking about the coding!

@aplymill7 aplymill7 Sep 24, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Okay yeah this makes sense, let's discuss this in the next meeting. Especially because we were going to discuss MVC anyways!

"""
Returns how many points will be measured during experiment from the config data.

Parameters
----------
config_data : dict
The configuration data

Returns
-------
step_number : int
How many points will be measured during the experiment
"""

step_number = config_data["general_parameters"]["step_number"]

return step_number


def get_adv_params(adv_config_data):
"""
Returns parameters that shouldn't be altered by beginning users largely related to calibration from the config data.

Parameters
----------
config_data : dict
The configuration data

Returns
-------
conversion_factor : float
Value to convert output number to real voltage measured.
setpoint_adjuster: float
Value to shift the starting scheduled voltage to match the input voltage.
shunt_resistor: float
Value to adjust for internal resistance largely set by shunt resistor to measure current passed.
time_step: float
Minimum time to execute read/write cycle.
average_number: int
Number of times the measurement at each point will be performed.
"""

conversion_factor = adv_config_data["advanced_parameters"]["conversion_factor"]
set_gain = adv_config_data["advanced_parameters"]["setpoint_gain"]
set_offset = adv_config_data["advanced_parameters"]["setpoint_offset"]
Expand All @@ -119,6 +275,7 @@ def get_adv_params(adv_config_data):
time_step,
average_number,
)

def check_config_inputs(arg):
"""
Checks that all the data that should be numerical from that config
Expand All @@ -133,8 +290,8 @@ def check_config_inputs(arg):
_______
is_number: Boolean
Value is True if the arg is a number, False if not.

"""

try:
return isinstance(float(arg), float)
except:
Expand Down
4 changes: 2 additions & 2 deletions pytentiostat/plotter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

def plot_initializer(config_data):
exp_type = pytentiostat.config_reader.get_exp_type(config_data)
exp_time = pytentiostat.config_reader.get_exp_time(config_data)
exp_duration = pytentiostat.config_reader.get_exp_duration(config_data)

times = []
voltages = []
Expand All @@ -18,7 +18,7 @@ def plot_initializer(config_data):

# This is just for testing
if exp_type == "CA":
axes.set_xlim(0, 2 * exp_time)
axes.set_xlim(0, 2 * exp_duration)

# Let's switch commands based on experiment run
if exp_type == "LSV":
Expand Down