0

An experienced VBA programmer here, that is starting the delve into Python OOP. I fear it is so simple, that I am having issues finding an answer without asking for help.

I have written the following code: 

#Import packages
import openpyxl as xl
import os

class DataExtract:

#Initialize the class
def __init__(self,wb):
    self.wb = wb

#Set class method to return sheet for named range
@classmethod
def rng_sht(cls,dest):
    for title, coord in dest:
        return(title)

#Set class method to return cell for named range
@classmethod
def rng_coord(cls,dest):
    for title, coord in dest:
        return(coord)

#Set class method to retun value of named range
@classmethod
def rng_val(cls,rng):

    #Define destinations
    dest = wb.get_named_range(rng).destinations

    #Retrieve sheet
    sht = DataExtract.rng_sht(dest)
    coord  = DataExtract.rng_coord(dest)

    #Return value       
    return()


#Define workbook
wb = 'Test_WB'

#Initiate class
wb_cur = DataExtract(wb)

#Find temp for current sheet
Temp = wb_cur.rng_val('Temp')

I'm aware that my indentation is incorrect.

The issue that I am having is that when I call the rng_val class method, it is only returning the current value for the first method I call within (in this case, the "sht"). When I inactivate the "sht" line, the "coord" line functions correctly.

I suspect the issue is likely due to how I am calling class methods or how I have structured the class, but I am not sure.

Update

I have updated the code with feedback from all of you, with my script below. I am still having errors with exiting the loop in the rng_val class, which Max suggested yield to resolve. I attempted to fix to no avail.

#Import packages
import openpyxl as xl
import os

class DataExtract:

    #Initialize the class
    def __init__(self,wb):
        self.wb = wb

    #Set class method to return sheet for named range
    @classmethod
    def rng_sht(cls,dest):
        for title, coord in dest:
            return title

    #Set class method to return cell for named range
    @classmethod
    def rng_coord(cls,dest):
        for title, coord in dest:
            return coord

    #Set class method to retun value of named range
    @classmethod
    def rng_val(cls,wb,rng):

        #Define destinations
        dest = wb.get_named_range(rng).destinations

        #Retrieve sheet
        sht = cls.rng_sht(dest)
        coord  = cls.rng_coord(dest)

        print(sht)
        print(coord)

        #Return value       
        return 1


path = 'C:\\Users\\User\\Desktop\\Python\\PracFiles\\'

#Loop through workbooks in a given folder
for i in os.listdir(path):

    #Define workbook
    wb = xl.load_workbook(path + i,data_only=True)

    #Find temp for current sheet
    Temp = DataExtract.rng_val(wb,'Temp')
halfer
  • 19,824
  • 17
  • 99
  • 186
Diedrich
  • 184
  • 3
  • 8
  • 4
    If you are aware - why not fix it? – Patrick Artner Feb 18 '18 at 07:33
  • Issue with the indentation when copying over. Given the simplicity of the code, I don't suspect anyone will have any issues following. – Diedrich Feb 18 '18 at 07:35
  • 3
    But if we try to reproduce your problem then we have to correct the indentation, just because you can't be bothered. – cdarke Feb 18 '18 at 07:39
  • 2
    Well the next python newbie finding this question with your problem which seems to be his as well will be confused - maybe revisit [how to ask](https://stackoverflow.com/help/how-to-ask) and [mvce](https://stackoverflow.com/help/mcve), Python needs indentation and not having it introduces more errors in your code that we have to fix. – Patrick Artner Feb 18 '18 at 07:40
  • 2
    in `rng_val` you have a `return` within a loop, wich causes the loop to exit during the first iteration. You may want to have a look at the `yield` keyword. – MaxPowers Feb 18 '18 at 07:44
  • Code updated. Thanks Max, I'll try now. Cheers – Diedrich Feb 18 '18 at 07:45
  • Mabye also have a look at [meaning-of-classmethod-and-staticmethod-for-beginner](https://stackoverflow.com/questions/12179271/meaning-of-classmethod-and-staticmethod-for-beginner) and fix the missing `self.`/`cls.` in your classes method. Your classmethods kindof look as if they should be instancemethods instead. – Patrick Artner Feb 18 '18 at 07:49
  • Hi Max, I tried using the yield keywork in my loops, then converted the object to a string using the .join method and I am having the same issue. Why would a loop in the different class method exit the initiate class method calling it? – Diedrich Feb 18 '18 at 08:48
  • I've wrapped my head around the 'yield' keyword a bit more now. Basically, when I call the "dest" object once, it erases. – Diedrich Feb 18 '18 at 09:14

1 Answers1

0

There are several issues I can see that may have to do with your inexperience with Python OO.

  1. The empty return statements cannot have parenthesis after them. That would cause the function to return an empty tuple instead of nothing.
  2. If you plan on having your function return a value, place the value (or variable) immediately after the return statement like this: return coord. return on its own will just exit the function.

  3. The first parameter in any class method contains the instance of that class when called via an object. You called it self in the constructor and cls in the other methods. They are the same thing. You haven't been using it in your code in places it looks like it should be. See below:

    sht = DataExtract.rng_sht(dest)
coord  = DataExtract.rng_coord(dest)

That would call the function rng_sht statically passing dest as an instance of DataExtract which I'm near certain isn't intended. What you should do instead is use cls.rng_sht(dest) to references the object instance. Also, you cannot access class fields by just referencing them by themselves, like wb.get_named_range(rng) where wb is a field in DataExtract. Instead reference it via cls like cls.wb.get_named_range(rng)

Yserbius
  • 1,375
  • 12
  • 18