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 Default Subclass Properties in checkUnset #475

Merged
merged 5 commits into from
Feb 2, 2023

Conversation

lawrence-mbf
Copy link
Collaborator

Fixes #470

Motivation

See #470

How to test the behavior?

See #470

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

@lawrence-mbf lawrence-mbf changed the title auto-indent fillProps Fix Default Subclass Properties in checkUnset Nov 15, 2022
@lawrence-mbf
Copy link
Collaborator Author

Failing tests are fixed in #461

@bendichter
Copy link
Contributor

@lawrence-mbf do you know why these tests are failing? Would it be possible to fix them?

@lawrence-mbf
Copy link
Collaborator Author

@bendichter We're polluting our MatNWB install directory with schemas of other versions. All tests and tutorials should make sure when their code blocks call generateCore they add 'savedir', '.' (generateCore('savedir', '.')) as arguments (saves the generated class files in the current directory which is usually a temporary file) or nwbRead with 'ignorecache' (equivalent to load_namespace=False in pynwb).

As mentioned above, merging in #461 will fix it for the previously added tutorials.

@yarikoptic
Copy link

yarikoptic commented Jan 11, 2023

What is the plan for this PR - to be finished up/merged/released?

@lawrence-mbf
Copy link
Collaborator Author

@yarikoptic It can be merged. As mentioned, the test failures are already fixed upstream. Would you say that you approve of these changes?

Remove dependency on defined class names and instead operate on all properties.
Originally was all-or-nothing. Now detects if a Set already exists and appends instead
when that occurs.
@lawrence-mbf lawrence-mbf force-pushed the 470-subclass-default-type branch from 03ef958 to a460c09 Compare January 11, 2023 15:55
@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

❗ No coverage uploaded for pull request base (master@cf16d37). Click here to learn what that means.
The diff coverage is 91.74%.

@@            Coverage Diff            @@
##             master     #475   +/-   ##
=========================================
  Coverage          ?   87.09%           
=========================================
  Files             ?      128           
  Lines             ?     5263           
  Branches          ?        0           
=========================================
  Hits              ?     4584           
  Misses            ?      679           
  Partials          ?        0           
Impacted Files Coverage Δ
+file/Attribute.m 100.00% <ø> (ø)
+file/Dataset.m 97.26% <ø> (ø)
+file/Group.m 95.00% <ø> (ø)
+file/Link.m 100.00% <ø> (ø)
+file/fillClass.m 100.00% <ø> (ø)
+types/+util/parseConstrained.m 67.74% <67.74%> (ø)
+file/fillConstructor.m 95.72% <95.72%> (ø)
+file/fillProps.m 95.08% <96.55%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bendichter bendichter merged commit 16a5411 into master Feb 2, 2023
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.

[Bug]: Unexpected attributes with nwbRead Dandiset 000115
3 participants