open-reaction-database / ord-schema

Schema for the Open Reaction Database
https://open-reaction-database.org
Apache License 2.0
95 stars 27 forks source link

Macros for creating solutions and workups #599

Closed brilee closed 3 years ago

codecov[bot] commented 3 years ago

Codecov Report

Merging #599 (66388ca) into main (08a9bb6) will increase coverage by 0.79%. The diff coverage is 98.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #599      +/-   ##
==========================================
+ Coverage   76.48%   77.28%   +0.79%     
==========================================
  Files          18       19       +1     
  Lines        1956     1994      +38     
  Branches      485      493       +8     
==========================================
+ Hits         1496     1541      +45     
+ Misses        313      309       -4     
+ Partials      147      144       -3     
Impacted Files Coverage Δ
ord_schema/macros/solutions.py 97.29% <97.29%> (ø)
ord_schema/message_helpers.py 85.60% <100.00%> (+1.31%) :arrow_up:
ord_schema/units.py 92.59% <100.00%> (+2.42%) :arrow_up:
brilee commented 3 years ago

@skearnes tests passing now

brilee commented 3 years ago

Looks like pylint picked up two new rules we are now violating: W1514 (open() must be used with explicit encoding) and C0209 - use f-strings instead of .format(). what do you want to do about these? The first seems fix-worthy, the second kind of annoying.

skearnes commented 3 years ago

Looks like pylint picked up two new rules we are now violating: W1514 (open() must be used with explicit encoding) and C0209 - use f-strings instead of .format(). what do you want to do about these? The first seems fix-worthy, the second kind of annoying.

Those both seem like things that should be fixed. If there are places where f-strings don't make sense, we can disable them inline.

brilee commented 3 years ago

added tests / resolve comments

brilee commented 3 years ago

currently debugging the test_notebooks failure.

Traceback (most recent call last): File "/usr/share/miniconda/lib/python3.7/site-packages/traitlets/traitlets.py", line 537, in get value = obj._trait_values[self.name] KeyError: 'kernel_dirs'

...

File "/usr/share/miniconda/lib/python3.7/os.py", line 223, in makedirs mkdir(name, mode) FileExistsError: [Errno 17] File exists: '/home/runner/.ipython'

I believe this is caused by Treon running multiple jupyter kernels at once, and there is likely a race condition where each kernel sees that the lazily populated kernel_dirs traitlet doesn't exist and then tries to create it...

brilee commented 3 years ago

this recent commit seems like alikely culprit: https://github.com/ipython/ipython/commit/7a20999bb2097a781b81e3098ba3e379cdd6b14e#diff-3adb2b67c2d8a4ccf260824b6391483e94429fcd20e547c4729bb5a55855eae9

brilee commented 3 years ago

in the meantime, I reran the test and it seems to have randomly not triggered the race condition.