Skip to content

Don't duplicate the elements in dir#14023

Merged
LysandreJik merged 1 commit intomasterfrom
duplicate_in_dir
Oct 16, 2021
Merged

Don't duplicate the elements in dir#14023
LysandreJik merged 1 commit intomasterfrom
duplicate_in_dir

Conversation

@sgugger
Copy link
Copy Markdown
Collaborator

@sgugger sgugger commented Oct 15, 2021

What does this PR do?

There is a tiny bug in the way the __dir__ method of our lazy modules is implemented. To see it run:

import transformers
dir(transformers.models.bert)

which will give a list ending with

[...,
 'configuration_bert',
 'configuration_bert',
 'load_tf_weights_in_bert',
 'modeling_bert',
 'modeling_flax_bert',
 'modeling_tf_bert',
 'tokenization_bert',
 'tokenization_bert',
 'tokenization_bert_fast'
]

You can already see some elements are duplicated, and accessing some other submodules (like the modeling ones) will grow the list of duplicates.

This comes because we always add all the submodules to the __dir__ even if they are already there (the fact they are there or not depends on whether some element of the submodule was accessed to, because of the lazy init).

This PR fixes that.

Thanks to @NielsRogge for reporting the bug!

@sgugger sgugger requested a review from LysandreJik October 15, 2021 13:53
Copy link
Copy Markdown
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@LysandreJik LysandreJik merged commit 968ae57 into master Oct 16, 2021
@LysandreJik LysandreJik deleted the duplicate_in_dir branch October 16, 2021 00:09
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