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 tasks circular import #1296

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

jeswan
Copy link
Collaborator

@jeswan jeswan commented Mar 19, 2021

Importing BatchMixin in primary previously imported all tasks resulting in a circular import with tokenization_normalization(). This PR fixes the circular imports by removing import * in tasks __init__.py.

@jeswan jeswan force-pushed the js/bugfix/circular_imports branch 2 times, most recently from d2522e9 to 4cea118 Compare March 19, 2021 15:06
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #1296 (7e6a26b) into js/feature/easy_add_model (9a7aa78) will increase coverage by 0.01%.
The diff coverage is 53.65%.

Impacted file tree graph

@@                      Coverage Diff                      @@
##           js/feature/easy_add_model    #1296      +/-   ##
=============================================================
+ Coverage                      49.82%   49.83%   +0.01%     
=============================================================
  Files                            163      162       -1     
  Lines                          11166    11170       +4     
=============================================================
+ Hits                            5563     5567       +4     
  Misses                          5603     5603              
Impacted Files Coverage Δ
jiant/proj/simple/runscript.py 45.16% <ø> (ø)
jiant/tasks/evaluate/core.py 33.33% <41.37%> (+0.43%) ⬆️
jiant/proj/main/components/container_setup.py 44.66% <80.00%> (+0.54%) ⬆️
jiant/proj/main/tokenize_and_cache.py 39.13% <83.33%> (+0.89%) ⬆️
jiant/proj/main/modeling/model_setup.py 23.14% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a7aa78...7e6a26b. Read the comment docs.

import jiant.tasks.lib.templates.squad_style.core as squad_style
import jiant.tasks.lib.templates.squad_style.utils as squad_style_utils

from jiant.tasks.retrieval import AbductiveNliTask
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed, could we remove the individual imports and just do import jiant.tasks.retrieval as tasks retrieval?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Switched to this style of import.

@jeswan jeswan force-pushed the js/bugfix/circular_imports branch from 4cea118 to 7e6a26b Compare March 25, 2021 16:35
@jeswan jeswan merged commit 4ddc5ac into js/feature/easy_add_model Mar 25, 2021
@jeswan jeswan deleted the js/bugfix/circular_imports branch March 25, 2021 16:38
jeswan added a commit that referenced this pull request May 4, 2021
* Update to Transformers v4.3.3 (#1266)

* use default return_dict in taskmodels and remove hidden state context manager in models.

* return hidden states in output of model wrapper

* Switch to task model/head factories instead of embedded if-else statements (#1268)

* Use jiant transformers model wrapper instead of if-else. Use taskmodel and head factory instead of if-else.

* switch to ModelArchitectures enum instead of strings

* Refactor get_output_from_encoder() to be member of JiantTaskModel (#1283)

* refactor getting output from encoder to be member function of jiant model

* switch to explicit encode() in jiant transformers model

* fix simple runscript test

* update to tokenizer 0.10.1

* Add tests for flat_strip() (#1289)

* add flat_strip test

* add list to test cases flat_strip

* mlm_weights(), feat_spec(), flat_strip() if-else refactors (#1288)

* moves remaining if-else statments to jiant model or replaces with model agnostic method

* switch from jiant_transformers_model to encoder

* fix bug in flat_strip()

* Move tokenization logic to central JiantModelTransformers method (#1290)

* move model specific tokenization logic to JiantTransformerModels

* implement abstract methods for JiantTransformerModels

* fix tasks circular import (#1296)

* Add DeBERTa (#1295)

* Add DeBERTa with sanity test

* fix tasks circular import

* [WIP] add deberta tests

* Revert "fix tasks circular import"

This reverts commit f924640.

* deberta tests passing with transformers 6472d8

* switch to deberta-v2

* fix get_mlm_weights_dict() for deberta-v2

* update to transformers 4.5.0

* mark deberta test_export as slow

* Update test_tokenization_normalization.py

* add guide to add a model

* fix test_expor_model tests

* minor pytest fixes (add num_labels for rte, overnight flag fix)

* bugfix for simple api notebook

* bugfix for #1310

* bugfix for #1306: simple api notebook path name

* squad running

* 2nd bugfix for #1310: not all tasks have num_labels property

* simple api notebook back to roberta-base

* run test matrix for more steps to compare to master

* save last/best model test fix

Co-authored-by: Jesse Swanson <js11133Wnyu.edu>
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