Skip to content

Conversation

@amzhao16
Copy link
Collaborator

@amzhao16 amzhao16 commented Apr 9, 2025

Add get_types function, where the user can input a component (i.e. "load") and see a list of all possible default parameter sets for that component. Optionally, they can set show_ranges = True which prints the entire config of each type as well.

Add corresponding test suite for this user query functionality.

Also, made a fix to make the init parameters more efficient: just one class variable "self.count" that stores all counts instead of 5 different count variables.

@amzhao16 amzhao16 requested a review from sarahmish April 9, 2025 20:48
Copy link
Contributor

@sarahmish sarahmish left a comment

Choose a reason for hiding this comment

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

Hi @amzhao16, thank you for opening up this issue! I am thinking about this implementation and have a few suggestions:

  • creating counts does help us have one source for counts instead of investigating the variables of all loads, lines, etc.
  • this, however, removes the readability of the PyGridSim object where now I need two steps into finding how many transformers or loads there are.
  • I'm wondering if we can look into a solution that bridges both

Here is an example using __dict__ to get the attributes of a class.

In [2]: class Example:
   ...:     def __init__(self):
   ...:         self.num_loads = 0
   ...:         self.num_lines = 0
   ...:         self.num_pv = 0

In [3]: e = Example()

In [4]: e.__dict__
Out[4]: {'num_loads': 0, 'num_lines': 0, 'num_pv': 0}

or a safer implementation would be to implement a function:

def get_counts(self):
    return {
        "loads": self.num_loads,
        "lines": self.num_lines,
        ...etc
    }

@amzhao16
Copy link
Collaborator Author

Hi @amzhao16, thank you for opening up this issue! I am thinking about this implementation and have a few suggestions:

  • creating counts does help us have one source for counts instead of investigating the variables of all loads, lines, etc.
  • this, however, removes the readability of the PyGridSim object where now I need two steps into finding how many transformers or loads there are.
  • I'm wondering if we can look into a solution that bridges both

Here is an example using __dict__ to get the attributes of a class.

In [2]: class Example:
   ...:     def __init__(self):
   ...:         self.num_loads = 0
   ...:         self.num_lines = 0
   ...:         self.num_pv = 0

In [3]: e = Example()

In [4]: e.__dict__
Out[4]: {'num_loads': 0, 'num_lines': 0, 'num_pv': 0}

or a safer implementation would be to implement a function:

def get_counts(self):
    return {
        "loads": self.num_loads,
        "lines": self.num_lines,
        ...etc
    }

I am a bit confused on how these methods reduce the number of calls we would have to make. With the dictionary, wouldn't we still need to do e.dict['num_loads'], which is I think as complicated and less readable than doing something like self.counts['num_loads'].

The function implementation still necessitates us to initiate all the variables, which is what I was trying to avoid (self.num_loads, self.num_lines, etc.) but to fetch counts I still have to go through two steps, like get_counts["loads"].

I can switch back to just using self.num_loads, self.num_lines, etc. directly if that's better, but am a bit confused on how these suggestions would mitigate the concern. Thanks, let me know if I'm missing something

@sarahmish
Copy link
Contributor

It doesn't reduce the number of calls. I want to maintain the readability of the PyGridSim object, by keeping the attributes clear. If we have one attribute called count, then (1) it's like re-implementing __dict__; (2) we no longer have an idea of what the object has as attributes, we need count.keys() to figure that out.

@amzhao16
Copy link
Collaborator Author

It doesn't reduce the number of calls. I want to maintain the readability of the PyGridSim object, by keeping the attributes clear. If we have one attribute called count, then (1) it's like re-implementing __dict__; (2) we no longer have an idea of what the object has as attributes, we need count.keys() to figure that out.

Ah I understand. Would it be better to just keep the variables separate then (as it originally was), to keep the readability? I wanted to reduce the number of lines but am thinking the simplest way to preserve this would just be to have them separately. (I didn't think about the fact that users can also just do like circuit.num_loads, but given this information, probably is best to just keep it separate?)

@amzhao16 amzhao16 requested a review from Chrisschmit April 15, 2025 15:36
@amzhao16 amzhao16 requested a review from sarahmish April 16, 2025 00:21
Copy link
Contributor

@sarahmish sarahmish left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @amzhao16!

@amzhao16 amzhao16 merged commit 974caa1 into main Apr 16, 2025
36 checks passed
@amzhao16 amzhao16 self-assigned this Apr 28, 2025
@amzhao16 amzhao16 added the enhancement New feature or request label Apr 28, 2025
@amzhao16 amzhao16 added this to the 0.1.0 milestone Apr 28, 2025
@sarahmish sarahmish changed the title Add support of user query functions, change in class variables Support User Query Functions May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants