Conversation
register_evse now edits _voltage and _phase_angles elements if an EVSE is re-registered instead of appending.
I.e. by using add_constraint instead of setting directly.
This avoids a mypy error regarding setting object attributes via accessors.
Fix type of algo in TestSortedAlgorithms to Optional[SortedSchedulingAlgo]. Remove redundant type definitons. Check that algo is not None before running tests, raising informative error if this is not the case.
This satisfies mypy's type checker.
This allows for tests to set the estimator without triggering a type checker error.
The objects being del'd are not defined anyway.
This keeps with LSP (consistency with Interface.is_feasible).
…laced with new names in case types actually do change. Change signature of _to/_from_dict to allow for optional context or loaded dicts. Type session_generator output to TypedDict so mypy can do more specific checks. Add typing_extensions to setup requirements. Conditionally import forward references using TYPE_CHECKING.
Add error to constraint_currents if no constraints have been added.
SortFuncCallback allows for SortedSchedulingAlgo._sort_fn to be set with a Callable without mypy thinking it's a bound method. Allow SessionInfo constructor to take ndarrays as inputs for min/max_rates. Edit sessions types in test_preprocessing.py to reflect if it's SessionDict or SessionInfo.
Remove del statements for type checker to pass (Discuss). Add checks for float and int in scalar max/min rates checks.
Add comments for type checker to ignore object returns from pydoc.locate and _build_from_id.
# Conflicts: # acnportal/acnsim/network/tests/test_network.py
| # If there are no constraints, throw an error as this function is not well- | ||
| # defined. | ||
| # TODO: Test the no constraint case for is_feasible and constraint_currents. | ||
| if self.constraint_matrix is None: |
There was a problem hiding this comment.
@zach401 Does this look good for the None case? If so I'll write a test
There was a problem hiding this comment.
I think we should try to support the no constraint case. I thought this was already covered, but maybe not?
There was a problem hiding this comment.
is_feasible returns True if called when no constraints are added. It's constraint_current that throws an error if called without constraints since there aren't any currents to constrain. But we could also just return an empty NumPy array. I think that's our best bet if we want to avoid throwing an error. I think not erroring, in this case, is fine as long as we're clear about the behavior in the docstring. @zach401 sound good?
There was a problem hiding this comment.
To clarify, I'd return np.array([]) if self.constraint_matrix is None (and add a test).
| self.max_recompute = scheduler.max_recompute | ||
| self.scheduler.register_interface(interface_type(self)) | ||
|
|
||
| def generate_interface(self, interface_type: type) -> Interface: |
There was a problem hiding this comment.
@zach401 For cases when an input scheduler is not given to the Simulator, we still need a way to get an interface to a simulator (for, e.g. RL training in which a simulation is stepped). If this looks good I'll write a test
There was a problem hiding this comment.
What do you think about get_interface() instead?
I also don't understand the doc string, why does it matter if scheduler is None? This function doesn't case what the scheduler is, right?
There was a problem hiding this comment.
I had anticipated that it would only ever be called when scheduler is None because otherwise, the Interface is accessible through scheduler.interface. That being said, the method itself doesn't require that, so I'll update the docstring and change to get_interface.
| if not isinstance(loaded_event, Event): | ||
| warnings.warn( | ||
| f"Got an element of type {loaded_event} when loading ev_history," | ||
| f"which is not an instance of EV or a subclass. Loaded " |
There was a problem hiding this comment.
TODO @sunash needs to say "...instance of Event or a subclass"
| if self.max_rate_estimator is not None: | ||
| self.max_rate_estimator.register_interface(interface) | ||
|
|
||
| def register_max_rate_estimator(self, interface: Interface) -> None: |
There was a problem hiding this comment.
TODO @sunash This needs to be implemented, right now it's a register_interface clone
| # If there are no constraints, throw an error as this function is not well- | ||
| # defined. | ||
| # TODO: Test the no constraint case for is_feasible and constraint_currents. | ||
| if self.constraint_matrix is None: |
There was a problem hiding this comment.
I think we should try to support the no constraint case. I thought this was already covered, but maybe not?
| self.max_recompute = scheduler.max_recompute | ||
| self.scheduler.register_interface(interface_type(self)) | ||
|
|
||
| def generate_interface(self, interface_type: type) -> Interface: |
There was a problem hiding this comment.
What do you think about get_interface() instead?
I also don't understand the doc string, why does it matter if scheduler is None? This function doesn't case what the scheduler is, right?
# Conflicts: # .travis.yml # acnportal/acnsim/network/tests/test_network.py # setup.py
Ignore one error with battery_kwargs.
# Conflicts: # .travis.yml # acnportal/acnsim/events/event.py # acnportal/acnsim/simulator.py # acnportal/algorithms/tests/generate_test_cases.py # tutorials/lesson2_implementing_a_custom_algorithm.ipynb
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
We would need to get all the type hinting to pass and address the remaining comments. Also if resolving #106 yields a minimum version of python that has future.annotations, the type hints might use that package rather than typing. Either way we should address #106 before moving forward with this PR. |
…frame on the 1st constraint
Pd upgrade
Documents build/test commands, high-level architecture of acnsim (Simulator, Interface, ChargingNetwork, BaseAlgorithm, BaseSimObj serialization), and key style conventions from CONTRIBUTING.md. https://claude.ai/code/session_01FpQTLzRzzeVEdGU5LVLDps
It randomizes EV-to-station assignment (with a waiting queue), not constraint limit values. https://claude.ai/code/session_01FpQTLzRzzeVEdGU5LVLDps
- Remove incorrect "three main packages" count (there are five) - Replace # noqa (never used) with the actual # noinspection PyProtectedMember annotation https://claude.ai/code/session_01FpQTLzRzzeVEdGU5LVLDps
The EV/Battery/BaseEVSE bullets restated what class names and file paths already communicate. Per best practice, only include what Claude can't infer from reading code. https://claude.ai/code/session_01FpQTLzRzzeVEdGU5LVLDps
Resolves conflicts in type hints (Optional return types, Dict vs dict for Python 3.8 compatibility) and docstring formatting in favor of master's intentional improvements.
Sync dev with master (v0.3.3 + pandas/Python version updates)
…J4Ag5 Add comprehensive CLAUDE.md documentation
5 participants
This PR also includes functionality to allow for altering an EVSE after constraints have been added.