Skip to content
This repository was archived by the owner on Jan 29, 2024. It is now read-only.

[BBS-145] Rewriting the mining schema class #155

Merged
merged 7 commits into from
Nov 2, 2020
Merged

[BBS-145] Rewriting the mining schema class #155

merged 7 commits into from
Nov 2, 2020

Conversation

Stannislav
Copy link
Contributor

As noted in [BBS-145] we use the dataclass decorator for the SchemaRequest class, but these appeared in the standard library only in python 3.7. The reason that our tests didn't fail before is because we were lucky that some of our dependencies installed the dataclasses backport for python 3.6

The SchemaRequest class was replaces by a new class, MiningSchema, that offers a robust and user-friendly interface.

@Stannislav Stannislav requested review from jankrepl, EmilieDel and pafonta and removed request for jankrepl November 2, 2020 11:34
@Stannislav
Copy link
Contributor Author

I think that's the first time the test fail on packaging tests. I hope it's a Travis glitch.

Comment on lines -1118 to +1107
"display_name": "BBS-BBG",
"display_name": "Python 3",
"language": "python",
"name": "devel"
"name": "python3"
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I do not care, but did we not have a policy of git checkout -p of these irrelevant changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I must have missed that policy discussion. How should we do it?

And yes, I'm also annoyed by the diffs of the notebook.

Copy link
Contributor

Choose a reason for hiding this comment

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

I vaguely remember that @FrancescoCasalegno pointed this out for some of our previous PRs.

There seems to be some scripts that take out the output + metadata automatically, for example
jupyter/nbconvert#805

@@ -1129,7 +1116,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.7.4"
"version": "3.6.10"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor

@EmilieDel EmilieDel left a comment

Choose a reason for hiding this comment

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

LGTM! I just found a small typo. 😄

@Stannislav Stannislav merged commit 52ac4b4 into master Nov 2, 2020
@Stannislav Stannislav deleted the bbs_145 branch November 2, 2020 14:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants