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

Merge wh-questions library in its current state #502

Merged
merged 72 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 69 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
8172546
Added wh regression test files
olzama Aug 9, 2020
9238372
Updated regression test index with wh entries
olzama Aug 9, 2020
1043df2
added initial files
olzama Aug 9, 2020
fbb8809
Assorted changes added while developing the wh-library
olzama Aug 9, 2020
65b1d7e
no message
olzama Aug 9, 2020
7276b55
no message
olzama Aug 9, 2020
8fa8099
no message
olzama Aug 9, 2020
eb26d39
no message
olzama Aug 9, 2020
e8b305b
no message
olzama Aug 9, 2020
bb9cae7
in this case, probably list() was needed? I got confused.
olzama Aug 9, 2020
4e88d77
no message
olzama Aug 9, 2020
7f3b214
no message
olzama Aug 9, 2020
70f0308
no message
olzama Aug 9, 2020
313d6bc
no message
olzama Aug 9, 2020
3d08e70
no message
olzama Aug 9, 2020
d539bc5
no message
olzama Aug 9, 2020
7d391d8
no message
olzama Aug 9, 2020
81860df
no message
olzama Aug 9, 2020
5c8233d
no message
olzama Aug 9, 2020
41ddfd4
no message
olzama Aug 9, 2020
c706822
no message
olzama Aug 9, 2020
2a97cbe
no message
olzama Aug 9, 2020
e409ec3
no message
olzama Aug 9, 2020
3a18f67
no message
olzama Aug 9, 2020
ef0a1cf
no message
olzama Aug 9, 2020
2f9f222
no message
olzama Aug 9, 2020
de79427
no message
olzama Aug 9, 2020
4480eed
All regression tests run to completion (had to disable wh-rus and wh-…
olzama Aug 10, 2020
480c48a
Merge branch 'allow-skipped-tests' into merge-wh
olzama Aug 10, 2020
51157e0
Merge branch 'allow-skipped-tests' into merge-wh
olzama Aug 11, 2020
1bc7c0d
no message
olzama Aug 21, 2020
db1acc1
Items 12--13 are in fact ungrammatical (marked with a * in the txt-su…
olzama Aug 21, 2020
3244dbb
Verbs are no longer SPR-emtpy, so, need additional SPEC constraints o…
olzama Aug 21, 2020
3e12134
More SPEC constraints (because verbs are no longer SPR < > )
olzama Aug 21, 2020
c5b5611
More SPEC < > constraints (because verbs are no longer SPR < >)
olzama Aug 21, 2020
3419703
Merge branch 'trunk' into merge-wh
olzama Aug 21, 2020
46b064b
Interrogative-clause was not added in some cases.
olzama Aug 21, 2020
ac090d8
Skip a valence change test which was never indexed and seems to be mi…
olzama Aug 21, 2020
572e3c6
Minimal adverb-adposition test.
olzama Aug 21, 2020
7417c9c
no message
olzama Aug 25, 2020
3d5d631
This is now passing fine.
olzama Aug 25, 2020
96a1849
no message
olzama Aug 25, 2020
804cfdd
The only change in the MRS should be that now we no longer have ICONS…
olzama Aug 25, 2020
7a23532
The only change in the MRS should be that now we no longer have ICONS…
olzama Aug 25, 2020
4ee0d79
no message
olzama Aug 25, 2020
3f9fc62
Only < e2 info-str e15 > type of thing should be missing from the upd…
olzama Aug 26, 2020
58f83b6
Only < e2 info-str e15 > type of thing should be missing from the upd…
olzama Aug 26, 2020
a768134
Only < e2 info-str e15 > type of thing should be missing from the upd…
olzama Aug 26, 2020
f7b4f15
Only < e2 info-str e15 > type of thing should be missing from the upd…
olzama Aug 26, 2020
e25f423
Only < e2 info-str e15 > type of thing should be missing from the upd…
olzama Aug 26, 2020
2eeb3bd
All changes to profiles should just be the removal of < e2 info-str e…
olzama Aug 26, 2020
a66152a
All changes to profiles should just be the removal of < e2 info-str e…
olzama Aug 26, 2020
8ac4511
Somehow missed the ICONS on the subordinator lexical items again in m…
olzama Aug 26, 2020
3e5cfd6
All changes should be just the removal of < e2 info-str e20 > type of…
olzama Aug 26, 2020
78041fa
Missed this in the merge.
olzama Aug 26, 2020
8e9397f
All changes should be just the removal of < e2 info-str e20 > type of…
olzama Aug 26, 2020
8071f33
The only changes here should be added icons, of the < e2 non-focus x2…
olzama Aug 26, 2020
bdf9858
Most diffs should be the removal of < e2 info-str e20 >; however test…
olzama Aug 27, 2020
9f23592
Merge branch 'trunk' into merge-wh
olzama Aug 31, 2020
eb6e4f8
no message
olzama Aug 31, 2020
7c6c294
Merge branch 'merge-wh' of https://github.com/delph-in/matrix into me…
olzama Aug 31, 2020
971cb2e
Got rid of the "globals" idea for now; instead, use a function in cho…
olzama Aug 31, 2020
f00ec10
no message
olzama Aug 31, 2020
a4b0f49
no message
olzama Aug 31, 2020
e09a65e
no message
olzama Aug 31, 2020
8385f47
no message
olzama Aug 31, 2020
d3a6dc7
This used to crash on me... But I can no longer reproduce it, so, goi…
olzama Aug 31, 2020
cc36bdd
no message
olzama Aug 31, 2020
a478c25
no message
olzama Aug 31, 2020
3aa9a81
no message
olzama Sep 1, 2020
e3477c9
no message
olzama Sep 1, 2020
6c07b4e
no message
olzama Sep 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
81 changes: 77 additions & 4 deletions gmcs/choices.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,11 @@ def postparse_uprev(self):
self.convert_30_to_31()
if self.version < 32:
self.convert_31_to_32()
if self.version < 33:
self.convert_32_to_33()
if self.version < 34:
self.convert_33_to_34()




Expand Down Expand Up @@ -718,6 +723,29 @@ def has_adp_case(self, case = '', check_opt = False):
return False


def has_det_case(self, case = '', check_opt = False):
"""
Returns True iff the target language has inflecting determiners
which agree with nouns in CASE.
"""

for det in self.get('det'):
opt = det.get('opt')
if opt or not check_opt:
for feat in det.get('feat', []):
if self.has_case(feat, case):
return True

for det_pc in self.get('det-pc'):
for lrt in det_pc['lrt']:
for feat in lrt.get('feat', []):
if feat['name'] == 'case':
return True

return False



def has_optadp_case(self, case = ''):
"""
Returns True iff the target language has optional case-marking
Expand Down Expand Up @@ -767,6 +795,13 @@ def has_dirinv(self):
return 'scale' in self.choices


def has_gender(self):
return 'gender' in self.choices

def has_png(self):
return ('pernum' in self.choices
or 'number' in self.choices or 'gender' in self.choices)

def has_SCARGS(self):
"""
Returns True iff the target language requires the SC-ARGS feature,
Expand All @@ -786,6 +821,19 @@ def has_SCARGS(self):

return result

def has_diverse_ques_particles(self):
# First figure out if there are diverse particles:
oblig = 0
imp = 0
for qpart in self.get('q-particle'):
if qpart['wh'] == 'oblig':
oblig += 1
elif qpart['wh'] == 'imp':
imp += 1
return (oblig > 0 and imp > 0)




# patterns()
# Create and return a list containing information about the
Expand Down Expand Up @@ -1214,9 +1262,10 @@ def features(self):


# Questions
if 'q-infl' in self.choices:
features += [ ['question', 'plus|plus', '', 'verb', 'y'] ]

if 'q-infl' in self.choices and not 'wh-q-infl' in self.choices:
features += [ ['question', 'polar|polar', '', 'verb', 'y'] ]
elif 'q-infl' in self.choices and 'wh-q-infl' in self.choices:
features += [ ['question', 'polar|polar;wh|wh;both|both;no|no', '', 'verb', 'y'] ]
# Information Structure
infostr_values = 'focus|focus;topic|topic;contrast|contrast;semantic-focus|non-contrastive-focus;contrast-focus|contrastive-focus;aboutness-topic|non-contrastive-topic;contrast-topic|contrastive-topic;focus-or-topic|focus-or-topic;contrast-or-focus|contrast-or-focus;contrast-or-topic|contrast-or-topic;non-topic|non-topic;non-focus|non-focus;bg|background'
#mkg_values = 'fc|focus;tp|topic;fc-only|focus-only;tp-only|topic-only;fc-+-tp|focus-and-topic;non-tp|non-topic;non-fc|non-focus;unmkg|unmarking'
Expand Down Expand Up @@ -1326,7 +1375,7 @@ def features(self):
# convert_value(), followed by a sequence of calls to convert_key().
# That way the calls always contain an old name and a new name.
def current_version(self):
return 32
return 34

def convert_value(self, key, old, new, partial=False):
if key in self:
Expand Down Expand Up @@ -2292,6 +2341,30 @@ def convert_31_to_32(self):
if strat.get('possessum-type')=='non-affix':
if not strat.get('possessum-mark-order'):
strat['possessum-mark-order'] = hc

def convert_32_to_33(self):
if self.get('q-part') == 'on':
orth = self.get('q-part-orth')
self.delete('q-part-orth')
self['q-particle'] = ChoiceList(full_key='q-particle')
cdict = ChoiceDict(full_key = 'q-particle1')
Comment on lines +2349 to +2350
Copy link
Contributor

Choose a reason for hiding this comment

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

These full_keys look wrong, but I'm definitely not an expert on the current choices files (and I really need to finish getting rid of them 🙂).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean by "look wrong"?

Copy link
Member

Choose a reason for hiding this comment

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

In #472 (still a draft) I removed the full_keys() method but not the full_key initialization parameter, but I expect it, too, will go away in the end.

Style-wise the spaces around the = are not PEP-8 compliant (but so far I haven't been raising style issues because there are so many in the codebase already... these could be fixed in a style-fixes-only kind of PR).

Copy link
Contributor

Choose a reason for hiding this comment

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

@olzama Oh, now that I look at it again, I think I misunderstood what these were doing. These are top level values, so I guess they're correct.

@goodmami in the Choices redo (still not published...) that I've been working on, I have kept full_key. I don't think it's inherently bad (though sometimes used poorly), and it is used a lot in the code.

Copy link
Member

Choose a reason for hiding this comment

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

@dantiston Ok I did git grep full_key and yes, it's more prevalent than I thought. Consider merging #472 to get rid of full_keys() method, which was only (ab)used in the adnominal_possession library. I still think most uses of full_key are not great. I was hoping we'd just use TOML and use the decoded dict instead of a custom object, but if we're using full_key it might not be that simple.

cdict['orth'] = orth
self['q-particle'].append(cdict)


def convert_33_to_34(self):
"""
The feature question's value "plus" was renamed "polar".
"""
if self.get('q-infl') == 'on':
for cat in ['verb-pc']:
for pc in self.get(cat):
for lrt in pc['lrt']:
for feat in lrt['feat']:
if feat['name'] == 'question' and feat['value'] == 'plus':
feat['value'] = 'polar'


########################################################################
# FormData Class
# This Class acts like form data which would normally
Expand Down
25 changes: 24 additions & 1 deletion gmcs/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
'''



### Choices for values of checkboxes, radio buttons ###

ON = 'on' # Choice convention for checked box
YES = 'yes' # Choice convention for selected radio button

ORTH = 'orth' # Choice convention for orthographies associated with stems and affixes.

### Section names ###

WORD_ORDER = 'word-order' # Choices section associated with word order subpage
Expand All @@ -33,3 +33,26 @@
OV_ORDERS = ['sov', 'ovs', 'osv', 'v-final']
VFINAL = ['sov','osv','v-final']
VO_ORDERS = ['svo', 'vos', 'vso', 'v-initial']

### WH-QUESTIONS

IN_SITU = 'in-situ' # No question phrase fronting
EMB_INSITU = 'embed-insitu' # Embedded in situ questions (when fronting is also possible; that's rare)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have a standard for constants names, but I feel like some of these are hard to understand, and could be improved by being replaced with the full words, except like WH (which is a library name) and OBLIG, MULTI, and INFL which are pretty standard. This one in particular seems problematic for namespace collisions with Emily's initials 🙂

On a more meta level, I remember you suggesting using constants, and while in general I agree, I think constant naming is a problematic issue. Constants add a (often useful) layer of abstraction that can also be a layer of confusion with confusing or misleading names. Name spacing is also hard, and while I don't think it needs to be solved perfectly in this PR, it would be nice to have a strategy, for instance defining library constants in library files (my preferred), using prefixes for library-specific constants, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think constants must be used for the sake of minimizing typos (of which we have lots and lots, and occasionally it costs hours of painful debugging).

As for what they should be -- that is of course another matter. The familiar tension between line length and explicitness...

Copy link
Member

Choose a reason for hiding this comment

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

The comment helps, but I agree that constants (and variable names in general) should generally be more descriptive, and EMB frequently occurs in comments as Emily's initials so I was confused here, too. EMBED_IN_SITU is only 3 more characters.

Copy link
Contributor

Choose a reason for hiding this comment

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

@olzama definitely agree constants are great. I just think these names could be a little longer (as @goodmami suggests).

Copy link
Collaborator Author

@olzama olzama Sep 1, 2020

Choose a reason for hiding this comment

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

I renamed the EMB-one but note that we have lots of issues in the code with long lines and inconsistent indentation in the TDL as it is. Renaming constants should in fact also involve indentation/space/linebreak cleanup, which is a separate task. (And to continue hinting at something I think about this issue generally: I do not believe there is any hope for bringing a project like the Matrix to a consistent state, ever. Why? Because in order for sucn initiatives to lead, on average, to anything apart from lengthy discussions, a military-grade authority is needed, which I don't think will ever be present in the project, nor would any of us want it to be, I don't think. You might think right now: oh but it is Olga who is arguing over this, but trust me, if we try to institute any kind of protocol, you will soon find yourself arguing over something you didn't even expect you would.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think a major piece of infrastructure that is missing right now (that we should probably make an issue for since it’s not there yet) is PR hooks and pre-commit hooks. The way these things are managed at my work very well across Python, C++, PHP, JavaScript, and more, is when you submit a PR, the server tells you if you’re breaking the rules. You can break the rules if you get a reviewer to accept, but there’s lots of signals about style problems, test failures, typing problems, etc. Yes, they aren’t 100%, but I think Black’s philosophy about no-config reformatting is great because it removes a category of disagreement.

There’s of course a balance to find between writing styling rules and using them, but moving towards automated tooling and commit-blocking checks is exactly the “military-grade authority” that will allow an async project like this to maintain style over time.

Last note: I have seen people write linter rules to complain about invalid constant names, so it’s possible if we want to do it :-) (and there‘s probably something existing we could use)

Copy link
Collaborator Author

@olzama olzama Sep 1, 2020

Choose a reason for hiding this comment

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

I don't seem to be able to get my "military-grade authority" point across. What I mean is, it can only come from a person who has actual power over the project. Such as the project owner. This project has always emphatically exercised a very different philosophy.

People will always have different opinions on any particular style issue. There is nothing inherently right or wrong in any particular style guide. Subscribing to one is either a personal decision, in which case it does not make sense to bring that discussion into a collaborative project at all because it can only lead to endless arguing (during which real bugs will be overlooked), or it is a project-wide decision, but that is only possible if one person who is actually the owner and has actual power over everyone else (as in: if you don't agree, I fire you) ultimately imposes the decision on everybody. Maybe something like this will happen in the Matrix one day but for now I don't see it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That said, I personally wouldn't mind a tool which I could optionally run to get an idea about the degree to which my code complies with some style guide. I would probably run it and would probably even follow (some of) its suggestions. I would be very against anything that actually prevents commits though, in the context of this project. (Not that I have that military-grade say, either!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to note that we already seem to be spending more time arguing over style issues (in a meta-way) than, say, discussing analyses or other contentful aspects of the work. I realize it may seem that I am the instigator but (i) actually, I am not :) I push back on these things but I never initiate them; and (ii) just wait till a project member comes on board who has strong views on style which are different from Mike's and T.J.'s Or even an issue surfaces on which Mike and T.J. happen to disagree.

No no, I really am of the opinion that style should be kept firmly at bay and, come to think of it, not even come into CR (because it simply is too distracting).

Copy link
Collaborator Author

@olzama olzama Sep 1, 2020

Choose a reason for hiding this comment

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

To conclude my above thoughts, I would be for encouraging using an automated tool which highlights style locally according to some guide (PEP or whatever) and for more or less banning (refraining from discussing) style in code reviews.

MTRX_FRONT = 'front-matrix' # Question phrases fronting in matrix clauses
WH_QUE_PTCL = 'wh-q-part' # Question particle for constituent questions
WH_INFL = 'wh-q-infl' # Inflectional paradigm for constituent questions
MULTI = 'multi' # Multiple (e.g. fronting)
ALL_OBLIG = 'all-oblig' # All question phrases are fronted obligatorily
SG_OBLIG = 'single-oblig' # One question phrase is obligatorily fronted
SINGLE = 'single'
NO_MULTI = 'no-multi-ques' # No multiple questions in one clause
PIED = 'pied-pip' # Pied piping
PIED_ADP = 'pied-pip-adp' # Pied piping of specifically adpositions
OBL_PIP_NOUN = 'oblig-pied-pip-noun' # Obligatory pied piping of nouns
OBL_PIP_ADP = 'oblig-pied-pip-adp' # Obligatory pied piping of adpositions
MTRX_FR_OPT = 'matrix-front-opt' # Optionality of fronting in matrix clauses


### LEXICAL TYPE NAMES (SOME); see lexbase.py
WH_PRO = 'wh-pronoun-noun-lex'
INTER = 'inter' # Interrogative words flag
11 changes: 10 additions & 1 deletion gmcs/customize.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from gmcs import tdl
from gmcs.choices import ChoicesFile
from gmcs.linglib import adnominal_possession
from gmcs.linglib import adverbs_adpositions
from gmcs.linglib import agreement_features
from gmcs.linglib import argument_optionality
from gmcs.linglib import case
Expand All @@ -32,6 +33,7 @@
from gmcs.linglib import valence_change
from gmcs.linglib import verbal_features
from gmcs.linglib import word_order
from gmcs.linglib import wh_ques
from gmcs.linglib import yes_no_questions
from gmcs.utils import format_comment_block

Expand Down Expand Up @@ -484,6 +486,10 @@ def customize_matrix(path, arch_type, destination=None, force_dest=False):
# do on the other TDL files. NOTE: This needs to be called
# before customize_lexicon() and customize_inflection()
lexical_items.insert_ids(ch)
# Currently, yes-no questions need to go before the lexicon
# because the lexicon needs to know whether there are obligatory question particles,
# to determine the head on the question-embedding verb.
yes_no_questions.customize_yesno_questions(mylang, ch, rules, lrules, hierarchies,roots)

# The following might modify hierarchies in some way, so it's best
# to customize those components and only have them contribute their
Expand Down Expand Up @@ -522,9 +528,12 @@ def customize_matrix(path, arch_type, destination=None, force_dest=False):
valence_change.customize_valence_change(mylang, ch, lexicon, rules, lrules, hierarchies)
word_order.customize_word_order(mylang, ch, rules)
coordination.customize_coordination(mylang, ch, lexicon, rules, irules)
yes_no_questions.customize_yesno_questions(mylang, ch, rules, lrules, hierarchies)
clausalmods.customize_clausalmods(mylang, ch, lexicon, rules, roots, trigger)
clausalcomps.customize_clausalcomps(mylang,ch,lexicon,rules)
adverbs_adpositions.customize_adv_adp(ch,mylang,rules)
wh_ques.customize_wh_ques(mylang,ch,rules,roots)

# Service customization
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this comment mean?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't know. I doubt I wrote it; more likely it got reshuffled up or down from somewhere...

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it was introduced in c706822 and wasn't elsewhere before. Maybe you just forgot why you wrote it? In which case it's probably doing no good and can be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, sounds good!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed the comment; I think a comment is in fact needed there, it is just that perhaps "Service" is not good.

customize_punctuation(grammar_path)
customize_test_sentences(grammar_path)
customize_itsdb(grammar_path)
Expand Down
2 changes: 2 additions & 0 deletions gmcs/deffile.py
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ class MatrixDefFile:
'tense-aspect-mood':'Tense, Aspect and Mood', 'evidentials': 'Evidentials',
'other-features':'Other Features', 'sentential-negation':'Sentential Negation',
'coordination':'Coordination', 'matrix-yes-no':'Matrix Yes/No Questions',
'wh-q':'Constituent (Wh-) Questions',
'info-str':'Information Structure',
'arg-opt':'Argument Optionality',
'clausal-comp':'Clausal Complements',
Expand All @@ -565,6 +566,7 @@ class MatrixDefFile:
'evidentials': 'Evidentials',
'other-features':'OtherFeatures', 'sentential-negation':'SententialNegation',
'coordination':'Coordination', 'matrix-yes-no':'YesNoQ',
'wh-q':'WhQ',
'info-str':'InformationStructure',
'arg-opt':'ArgumentOptionality',
'clausal-comp':'ClausalComplements',
Expand Down
7 changes: 7 additions & 0 deletions gmcs/feature_type_use.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Attempt to flag specific features which were added to the grammar by a component.
# One such feature is INIT; it is not needed in all grammars but can be added by the Word Order library,
# the clausal complements library, the clausal modifiers library, and the Wh-questions library
# (and possibly other libraries).

USED_FEATURES = {'INIT':False}
USED_TYPES = {'qdet':False,'qpro':False,'qadv':False,'qverb':False}
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this idea in principle, but I don't think this will work in practice like this forever? The way regression testing happens right now with multiprocess/subprocess, I guess it should be fine, but if someone tried to customize two grammars with one Python instance, it wouldn't reset between customizations.

I think there's a lot of opportunity for refactoring the current system, and this sort of thing seems to fit into that, so I'll open an issue for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to start collecting some statistics about what features are used how. I did not think about it much in terms of doing it over multiple grammars simultaneously although it would of course be interesting. But even within one grammar is interesting enough.

Of course I only started it and didn't really do anything interesting with these dicts yet.

Copy link
Member

Choose a reason for hiding this comment

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

I think T.J. means that in a multithreaded (not multiprocess) use of the Matrix (which we do not currently do but in the future who knows) this global might be modified by different customizations in the same Python process, leading to incorrect results. But, as a side-point, if these are intended to be modified they should not be ALL_CAPS as that indicates they are constants.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(another reason to not use multithreading, as far as I am concerned :) ).

Copy link
Member

Choose a reason for hiding this comment

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

We're not using multithreading

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know :). Good :). (Sorry, I am just messing with ya at this point)

Loading