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

docs(api): Remove deprecated behavior from api docs #7828

Merged
merged 4 commits into from
May 27, 2021

Conversation

ethanfjones-zz
Copy link
Contributor

plate.wells() was used when plate.well() should be used. I replaced all instances of plate.wells() with plate.well()

Overview

Changelog

Review requests

Risk assessment

plate.wells() was used when plate.well() should be used. I replaced all instances of plate.wells() with plate.well()
@ethanfjones-zz ethanfjones-zz requested a review from a team as a code owner May 24, 2021 17:08
@codecov
Copy link

codecov bot commented May 24, 2021

Codecov Report

❗ No coverage uploaded for pull request base (edge@9733697). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 05a4513 differs from pull request most recent head e98f97d. Consider uploading reports for the commit e98f97d to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #7828   +/-   ##
=======================================
  Coverage        ?   83.49%           
=======================================
  Files           ?      336           
  Lines           ?    21347           
  Branches        ?        0           
=======================================
  Hits            ?    17823           
  Misses          ?     3524           
  Partials        ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9733697...e98f97d. Read the comment docs.

@@ -74,7 +74,7 @@ Loops in Python allow your protocol to perform many actions, or act upon many we

# range() starts at 0 and stops before 8, creating a range of 0-7
for i in range(8):
p300.distribute(200, reservoir.wells()[i], plate.rows()[i])
p300.distribute(200, reservoir.well()[i], plate.rows()[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

This should still be .wells().

@@ -99,7 +99,7 @@ The OT-2 pipettes can do some things that a human cannot do with a pipette, like

p300.pick_up_tip()

for well in reservoir.wells()[:4]:
for well in reservoir.well()[:4]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@@ -128,13 +128,13 @@ This example first spreads a diluent to all wells of a plate. It then dilutes 8
tiprack_2 = protocol.load_labware('opentrons_96_tiprack_300ul', 3)
reservoir = protocol.load_labware('usascientific_12_reservoir_22ml', 4)
p300 = protocol.load_instrument('p300_single', 'right', tip_racks=[tiprack_1, tiprack_2])
p300.distribute(50, reservoir['A12'], plate.wells()) # dilutent
p300.distribute(50, reservoir['A12'], plate.well()) # dilutent
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this example as well.


# loop through each row
for i in range(8):

# save the source well and destination column to variables
source = reservoir.wells()[i]
source = reservoir.well()[i]
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here

@@ -44,8 +44,8 @@ This accomplishes the same thing as the following basic commands:
p300 = protocol.load_instrument('p300_single', 'right', tip_racks=[tiprack_1])

p300.pick_up_tip()
p300.aspirate(100, plate.wells('A1'))
p300.dispense(100, plate.wells('B1'))
p300.aspirate(100, plate.well('A1'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, this is deprecated behavior. I'm wondering if we should just remove this completely from the docs to discourage users from using this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha so wells can only be used with distribute/transfer?

I only tested this with the aspirate and assumed the others were just a typo.

Copy link
Contributor

@SyntaxColoring SyntaxColoring May 24, 2021

Choose a reason for hiding this comment

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

The deprecated behavior is that the well name is passed as an argument to the .wells() function, instead of as an index into the list that it returns.

In other words, instead of this:

plate.wells('B1')

It should be one of these:

plate.wells()[1]  # I *think* index 1 is B1?
# or
plate.wells_by_name()['B1']
# Edit to add: or this one too
plate['B1']

Note that this method takes args for backward-compatibility, but use of args is deprecated and will be removed in future versions. Args can be either strings or integers, but must all be the same type (e.g.: self.wells(1, 4, 8) or self.wells(‘A1’, ‘B2’), but self.wells(‘A1’, 4) is invalid.

(from https://docs.opentrons.com/v2/new_protocol_api.html?highlight=wells#opentrons.protocol_api.labware.Labware.wells)

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Laura that our example code shouldn't be using the deprecated plate.wells(well_name) form.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I'm not getting notifications on my github comments for some reason.

The equivalent of plate.well('B1') is now -> plate['B1']. If a user wants to have a list of wells, they should simply use python indexing; i.e. plate.wells()[0:7] which would give the first column of a 96 well labware.

@ethanfjones, I'm not sure I follow Gotcha so wells can only be used with distribute/transfer?. You can only use a single well when using the "atomic" commands such as aspirate or dispense. You can use a single well or a list of wells for advanced commands such as distribute or transfer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Laura-Danielle and Mr. Syntax, thanks for the response.

Your explanation makes sense to me!

@Laura-Danielle Laura-Danielle self-requested a review May 27, 2021 22:08
@Laura-Danielle Laura-Danielle changed the title Change instances of wells to well docs(api): Remove deprecated behavior from api docs May 27, 2021
@SyntaxColoring SyntaxColoring self-requested a review May 27, 2021 22:31
@SyntaxColoring SyntaxColoring merged commit 42e58df into edge May 27, 2021
@SyntaxColoring SyntaxColoring deleted the ethanfjones-patch-1 branch May 27, 2021 22:32
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.

3 participants