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

Fix Conditional Circular Import #183

Open
beckydvn opened this issue Jun 1, 2022 · 0 comments
Open

Fix Conditional Circular Import #183

beckydvn opened this issue Jun 1, 2022 · 0 comments
Labels
bug Something isn't working

Comments

@beckydvn
Copy link
Collaborator

beckydvn commented Jun 1, 2022

I noticed that there are still some circular import errors between trace and observation, the strange thing being that if trace is being imported before observation then nothing happens but importing observation on its own causes a circular import.

My understanding is that this is because:

  • observation imports Observation from observation.py which imports from trace, which runs the imports from trace.py which imports Observation. So observation has a circular import built in.
  • trace imports from trace.py which imports Observation, which runs the imports for observation.py which imports State and Action but not Trace, so no direct circular import is caused.
  • Importing trace before observation fixes the circular import as trace.py (and the second Observation by proxy) won't need to be imported again.

Now the issue around circular imports was addressed in #143 and #145, so not sure if this is by design or if this use case was missed (naturally anything trace-like was always imported before anything observation-like as they would be used first). At the very least the user should get a more helpful message so they're not left scratching their head at an internal import error should they happen to import anything from observations first.

Additionally (and perhaps this should be a separate issue) but while we're on the topic of observations vs. traces, observations.print(view="details") crashes as the data structure isn't totally set up the same as TraceList, so maybe ObservationLists just shouldn't inherit print functionality?

@e-cal I think you designed most of the ObservationLists/TraceList functionality so maybe I'm just missing something, would like to know your thoughts on these 😃

@beckydvn beckydvn added the bug Something isn't working label Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant