search

corneliusroemer / pango_aliasor

Utility to alias and dealias pango lineages
MIT License
21 stars 6 forks source link

Adds URL to constructor #9

Open ciscorucinski opened 1 year ago

ciscorucinski commented 1 year ago

Description of proposed changes

It is intended to allow for #7 url proposal.

This proposal modified the existing constructor to take in required, named arguments. If you don't use the default constructor, then you must specify the argument you are setting Aliasor(alias_file=<file>). This changes the behavior of the existing file argument as you can no longer call Aliasor(<file>).

I added the ability to specify a url via Aliasor(alias_url=<url>).

The use of required, named arguments should be implemented if you want to add different input data such as str (which can then be implemented as Aliasor(alias_string=<str>)). This makes it very clear what the caller wants to do.

Related issue(s)

Testing

I added a test which takes in the default url currently provided, and tested that compress and uncompress logic still works correctly. I also updated the file constructor test to use the new required, named argument implementation.

ciscorucinski commented 1 year ago

I am using PyCharm, and the current test imports from this project don't work as-is.

I had to change from pango_aliasor.aliasor import Aliasor to from src.pango_aliasor.aliasor import Aliasor to remove import errors and get the tests to be recognized.

I did not include that in this pull request, but if you would like me to, I can add this change, too.