Skip to content

Conversation

@bernt-matthias
Copy link
Contributor

Just a simple minded idea to distinguish input and output files and directories.

Could be realized by subclasses as well if preferred.

Btw. the regular expressions like bool(ean)?, in(put)?, and out(put)? could be simplified by removing the optional part?

@multimeric
Copy link
Collaborator

Looks great! As you say, could you do this in one place either by adding a new parent class or a mixin? Secondly, could you add a simple test for this? I don't currently have a single place that I test the type inference, so I'd be happy for you to add a new top level test file, e.g. test/test_type_inference.py, and in there you can just add some simple unittests for infer_type() that tests your new features.

@multimeric multimeric linked an issue Jul 23, 2020 that may be closed by this pull request
@bernt-matthias bernt-matthias force-pushed the topic/guess-in-or-out branch 10 times, most recently from a2f2e79 to 87ed6cd Compare July 23, 2020 21:28
@bernt-matthias bernt-matthias force-pushed the topic/guess-in-or-out branch 3 times, most recently from c29a8c1 to d81e1f0 Compare July 24, 2020 11:39
@bernt-matthias
Copy link
Contributor Author

hooray .. tests are passing finally :)

@bernt-matthias bernt-matthias marked this pull request as draft July 27, 2020 07:38
@bernt-matthias bernt-matthias force-pushed the topic/guess-in-or-out branch from 5142d63 to e40300d Compare July 27, 2020 14:08
@bernt-matthias bernt-matthias marked this pull request as ready for review July 27, 2020 14:44
@bernt-matthias
Copy link
Contributor Author

This is ready from my side. I added the functionality that in the fallback case the string is checked if there are numbers (int/float) and return CliInteger/CliFloat.

@bernt-matthias
Copy link
Contributor Author

I guess we should merge #25 first and rebase this PR afterwards.

@multimeric
Copy link
Collaborator

I guess we should merge #25 first and rebase this PR afterwards.

Okay I've merged it, feel free to pull master in this branch.

@bernt-matthias bernt-matthias force-pushed the topic/guess-in-or-out branch from e40300d to 6e99ddc Compare July 27, 2020 16:12
@multimeric
Copy link
Collaborator

Hmm, you've resolved a few comments that you haven't actually changed yet in the code. Did you forget to push some changes?

@bernt-matthias bernt-matthias force-pushed the topic/guess-in-or-out branch from a4c2b8d to 7aaac45 Compare July 28, 2020 09:30
@multimeric
Copy link
Collaborator

Any chance you could finish this PR @bernt-matthias? I think we got really close to it being done. If you don't have time I can try, but it might take a little to get my head around it.

@bernt-matthias
Copy link
Contributor Author

I have a bit of time this week. Then I'm offline for one more week. What's left to do?

@multimeric
Copy link
Collaborator

Hmm, good point. I could have sworn there was something left to do, but the issues are resolved and all the tests are passing so maybe not? I'll double check.

@multimeric
Copy link
Collaborator

Some minor suggestions but overall I think this looks good.

@multimeric multimeric merged commit 436a8a6 into aCLImatise:master Aug 25, 2020
@multimeric
Copy link
Collaborator

Perfect, thanks for this!

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.

Automatic inference of "input outputs"

2 participants