Skip to content

Declare fields in PatchscopeBase - #35

Open
tvhong wants to merge 1 commit into
mainfrom
vhong/patchscope_base_fields2
Open

Declare fields in PatchscopeBase#35
tvhong wants to merge 1 commit into
mainfrom
vhong/patchscope_base_fields2

Conversation

@tvhong

@tvhong tvhong commented Apr 2, 2024

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
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

tvhong commented Apr 4, 2024

Copy link
Copy Markdown
Collaborator Author

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

tvhong commented Apr 6, 2024

Copy link
Copy Markdown
Collaborator Author

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

@shaheenahmedc

Copy link
Copy Markdown
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

tvhong commented Apr 26, 2024

Copy link
Copy Markdown
Collaborator Author

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.

2 participants