Skip to content

Conversation

@tvhong
Copy link
Collaborator

@tvhong tvhong commented Apr 2, 2024

Notes

PatchscopeBase references the fields inside the child class Patchscope.

This breaks Intellisense. So, I'm declaring all fields inside PatchscopeBase.

Also moves the Context classes to patchscope base to avoid circular dependency

Testing

pytest passed

Also moves the Context classes to patchscope base to avoid circular dependency
@shaheenahmedc
Copy link
Collaborator

This looks good to me. Do we need to change all imports of SourceContext and TargetContext in this PR, to account for this?

@tvhong
Copy link
Collaborator Author

tvhong commented Apr 4, 2024

Let me make obvs.patchscope a package. Then have 3 files in it: patchscope, patchscope_base, and contexts. That way, each file is small and users can still import from obvs.patchscope.

@tvhong
Copy link
Collaborator Author

tvhong commented Apr 6, 2024

I'll refactor after the FutureLens PR is merged to avoid conflict.

@shaheenahmedc
Copy link
Collaborator

@tvhong whenever you have some time, shall we get this merged? Haven't thought through if it will affect #48 and any scripts, but just in case

@tvhong
Copy link
Collaborator Author

tvhong commented Apr 26, 2024

Thanks for the reminder @shaheenahmedc. I'll wait for you to merge #48 & #52 and then rebase this change.

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