[FEATURE] modify is_sif,to_sif,sif4sci #79

KenelmQLH · 2021-08-31T06:16:31Z

Thanks for sending a pull request!
Please make sure you click the link above to view the contribution guidelines,
then fill out the blanks below.

Description

add params in sif to decided whether to check formulas
modify is_sif,to_sif,sif4sci to reduce time consumption

What does this implement/fix? Explain your changes.

add params in sif to decided whether to check formulas
modify is_sif,to_sif,sif4sci to reduce time consumption

Pull request type

[DATASET] Add a new dataset
[BUGFIX] Bugfix
[FEATURE] New feature (non-breaking change which adds functionality)
[BREAKING] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[STYLE] Code style update (formatting, renaming)
[REFACTOR] Refactoring (no functional changes, no api changes)
[BUILD] Build related changes
[DOC] Documentation content changes
[OTHER] Other (please describe):

Changes

EduNLP/SIF/parser/parser.py
EduNLP/SIF/sif.py
tests/test_sif/test_sif.py

Does this close any currently open issues?

Y, #37 ([FEATURE] Add an option for checking formulas in is_sif)

Any relevant logs, error output, etc?

N

Checklist

Before you submit a pull request, please make sure you have to following:

Essentials

PR's title starts with a category (e.g. [BUGFIX], [FEATURE], [BREAKING], [DOC], etc)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage and al tests passing
Code is well-documented (extended the README / documentation, if necessary)
If this PR is your first one, add your name and github account to AUTHORS.md

Comments

If this change is a backward incompatible change, why must this change be made.
Interesting edge cases to note here

codecov-commenter · 2021-08-31T06:21:46Z

Codecov Report

Merging #79 (13159a5) into dev (73232ad) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               dev       #79   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           46        46           
  Lines         1365      1371    +6     
=========================================
+ Hits          1365      1371    +6

Impacted Files	Coverage Δ
EduNLP/SIF/parser/parser.py	`100.00% <100.00%> (ø)`
EduNLP/SIF/sif.py	`100.00% <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 73232ad...13159a5. Read the comment docs.

tswsxk · 2021-09-01T01:54:59Z

It seems that this PR is relevant to issue #37 ?

KenelmQLH · 2021-09-01T02:59:50Z

examples need to be changed if the codes are approved

tswsxk · 2021-09-02T07:33:54Z

EduNLP/SIF/sif.py

+    return sif_item


 def sif4sci(item: str, figures: (dict, bool) = None, safe=True, symbol: str = None, tokenization=True,


When you sepecify some modes such as ast in tokenzation_params, is it possible to set check_formula as False?

It is ok, but tokenzation_params mainly work during 'tokenize', and check_formula is used in is_sif before 'tokenize'. Maybe it is better to use seperately for clear direction.
Or we can use it within 'safe':
when safe = 0, don't use is_sif and don't check anything
when safe = 1, use is_sif but don't check formula
when safe = 2, use is_sif and check formula

safe looks like a boolean variable, maybe safe_mode is a better choice?

Maybe we can also set a wise safe_mode?

tswsxk · 2021-09-04T05:52:09Z

EduNLP/SIF/sif.py

-        return True
-    return False
+        return True, item
+    return False, item_parser.text


Why return text?

tswsxk · 2021-09-04T05:53:30Z

EduNLP/SIF/sif.py



-def is_sif(item):
+def is_sif(item, check_formula=True):


Is sif should only return True or False. If it is necessary to modify the item for reducing duplicate opertaion in the following procedure, an extra argument is expected.

tswsxk · 2021-09-26T04:06:04Z

EduNLP/SIF/sif.py

+
+        True if check the validity of formulas in item
+        False if not check the validity of formulas in item, which is faster
+    cache: bool


cache is not intuitive, maybe return_parser is a better choice

tswsxk · 2021-09-26T04:06:46Z

EduNLP/SIF/sif.py

+    figures:
+        when it is a dict, it means the id-to-instance information for figures in 'FormFigureID{...}' format,
+        when it is a bool, it means whether to instantiate figures in 'FormFigureBase64{...}' format
+    safe_mode: int


maybe mode is better than safe_mode

tswsxk · 2021-10-08T09:04:34Z

EduNLP/SIF/sif.py



-def to_sif(item):
+def to_sif(item, check_formula=True, cache_parser: Parser = None):


cache_parser -> parser

KenelmQLH added 2 commits August 31, 2021 14:08

[FEATURE] modify is_sif,to_sif,sif4sci

654df05

[FEATURE]add params in sif to decided whether to check formulas

0a06e06

tswsxk linked an issue Sep 1, 2021 that may be closed by this pull request

[FEATURE] Add an option for checking formulas in is_sif #37

Closed

KenelmQLH self-assigned this Sep 1, 2021

KenelmQLH added the enhancement New feature or request label Sep 1, 2021

tswsxk requested changes Sep 2, 2021

View reviewed changes

KenelmQLH added 2 commits September 3, 2021 16:53

modify examples

ef6f7c4

[feature] modify params and comments in sif4sci

338e727

tswsxk requested changes Sep 4, 2021

View reviewed changes

KenelmQLH added 2 commits September 11, 2021 16:52

[FEATURE] change the usage of is_sif,to_sif,sif4sci

9b72e32

Merge remote-tracking branch 'upstream/dev' into feat_sif

5de3d0d

tswsxk requested changes Sep 26, 2021

View reviewed changes

KenelmQLH added 2 commits September 26, 2021 17:07

[FEATURE] change the param name in sif.py

8f2856f

[examples] fix the sif examples

24cc80b

tswsxk requested changes Oct 8, 2021

View reviewed changes

KenelmQLH and others added 2 commits October 10, 2021 12:44

change the param name in sif.py and update examples

1d3889b

Update sif.py

f82e186

tswsxk approved these changes Oct 10, 2021

View reviewed changes

KenelmQLH added 2 commits October 10, 2021 22:49

Merge from dev

25c4cf9

Merge from origin/dev

13159a5

KenelmQLH merged commit a294dc0 into bigdata-ustc:dev Oct 10, 2021

		return sif_item


		def sif4sci(item: str, figures: (dict, bool) = None, safe=True, symbol: str = None, tokenization=True,



		def to_sif(item):
		def to_sif(item, check_formula=True, cache_parser: Parser = None):

[FEATURE] modify is_sif,to_sif,sif4sci #79

[FEATURE] modify is_sif,to_sif,sif4sci #79

Uh oh!

Conversation

KenelmQLH commented Aug 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What does this implement/fix? Explain your changes.

Pull request type

Changes

Does this close any currently open issues?

Any relevant logs, error output, etc?

Checklist

Essentials

Comments

Uh oh!

codecov-commenter commented Aug 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tswsxk commented Sep 1, 2021

Uh oh!

KenelmQLH commented Sep 1, 2021

Uh oh!

tswsxk Sep 2, 2021

Choose a reason for hiding this comment

Uh oh!

KenelmQLH Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 4, 2021

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 4, 2021

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 26, 2021

Choose a reason for hiding this comment

Uh oh!

tswsxk Sep 26, 2021

Choose a reason for hiding this comment

Uh oh!

tswsxk Oct 8, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

KenelmQLH commented Aug 31, 2021 •

edited

Loading

codecov-commenter commented Aug 31, 2021 •

edited

Loading

KenelmQLH Sep 3, 2021 •

edited

Loading

tswsxk Sep 3, 2021 •

edited

Loading

tswsxk Sep 3, 2021 •

edited

Loading