-
Notifications
You must be signed in to change notification settings - Fork 9
Allow elements to be passed in to enable selective parsing #28
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
Conversation
TODO:
|
bridgepoint/prebuild.py
Outdated
|
||
|
||
def prebuild_model(metamodel): | ||
def prebuild_model(metamodel, parent_paths=[]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python has a somewhat unexpected behavior with mutable default values:
def append(number, number_list=[]):
number_list.append(number)
print(number_list)
return number_list
append(5) # expecting: [5], actual: [5]
append(7) # expecting: [7], actual: [5, 7]
append(2) # expecting: [2], actual: [5, 7, 2]
Perhaps it does not matter in this case, but I usually do it like this instead:
def prebuild_model(metamodel, parent_paths=None):
parent_paths = parent_paths or list()
...
Or you could use a default tuple() value instead, which is immutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will change it to have no default since the option parser will handle passing a default value if the user does not provide one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I will go with your suggestion since maybe that is more expressive when reading the code and does not assume the utility is run from the command line...
Package references are supported. Now the I also changed the implementation so that paths are represented as lists of individual segments instead of strings. This solves the weakness with using |
I am still not entirely sure what scenarios this PR cover, and I believe further documentation (on scenarios in /doc) is needed. However, before we commit time on review to identify missing corner cases, is this feature still relevant and desirable? |
I'm going to close this one for now and postpone work on it. It will be tracked further here: https://support.onefact.net/issues/12251 |
Selective parsing increases performance significantly when using a small bit of an external project. It also enables pulling in a dependency project for parsing without pulling in the whole dependency tree if the part you need does not rely on all the other dependencies.