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

Platform & parenthesis issues #104

Merged

Conversation

QuantumAli
Copy link
Contributor

This addresses both issues that were opened by me today. It should address the indentation error, and it also updates all of the examples to have the proper str(), and imports.
The read me was changed to reflect the changes of str() and import as well.

Copy link
Contributor

@vprusso vprusso left a comment

Choose a reason for hiding this comment

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

This addresses both issues that were opened by me today. It should address the indentation error, and it also updates all of the examples to have the proper str(), and imports. The read me was changed to reflect the changes of str() and import as well.

Just FYI, when you create a PR that refers to a specific issue that closes it, it should explicitly have in the description the following:

Closes #<NUMBER_OF_ISSUE>

This does two things. One, it's clear what issues are being referred to, and second, it automatically closes the issues in question once this PR is merged.

metriq/client.py Outdated
@@ -359,6 +359,13 @@ def task_get_names(self) -> List[Task]:
for r in response["data"]
]

@handler
def task_get_network(self) -> Task:
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably should not be here, right? This is specifically related to another issue and is not correct as implemented here.

@@ -0,0 +1,6 @@
from metriq import MetriqClient
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other comment regarding the network task graph, this example should not be added as it will not yield the expected result and is part of your other PR.

This branch was created from my main development branch, and thus had these extra things that weren't completely worked out
@QuantumAli
Copy link
Contributor Author

Closes #99 , Closes #100 

I do not know if I was supposed to fix those conflicts and merge the branches, but it had a couple of errors, and I fixed them. Ie with the indentation errors, and with the read me being updated.

This branch was taken off of my development branch which had all of those changes, and thus thats why we see the duplicates from the other PR

@vprusso
Copy link
Contributor

vprusso commented Aug 11, 2023

Thanks, @QuantumAli !

@vprusso vprusso merged commit c8f1928 into unitaryfund:development Aug 11, 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.

2 participants