CKAN Coding Standards #1547
CKAN Coding Standards #1547
Conversation
- Started linting.
- Continued linting.
- Continued linting.
- Continued linting.
- Added change log file.
- Removed dev requirements file.
- Circular import.
- Pyright changes,
- Pyright changes.
- Pyright changes.
- Pyright changes.
- Pyright changes.
- Pyright changes.
- Pyright changes.
- Pyright changes.
|
I kept the old migration scripts around as a record of the data schema changes in the past in case we ever need to compare data from backups with newer data. The other deletions should be fine but it's worth mentioning them in the changelog at least. |
@wardi I will just add a note to the changelogs for the removal of all this stuff. as we are cross training soon, I am trying to slim down all the actual code to operational things. So old migration scripts can just be found in git history if anyone needs them |
- Pyright changes.
- Pyright changes.
- Pyright changes.
- Pyright changes.
| """ | ||
| Produce a list of dataset ids and requested dates. Each package | ||
| id will appear at most once, showing the activity date closest | ||
| to since_date. Requested dates are preceeded with a "#" | ||
| """ | ||
| since_date = isodate(since_date, None) | ||
| since_datetime: Optional[datetime] = isodate(since_date, cast(Context, {})) |
There was a problem hiding this comment.
why is since_datetime optional? If we don't get a datetime wouldn't it be better to exit with an error here?
There was a problem hiding this comment.
isodate can return None
ckanext/canada/cli.py
Outdated
| # type_ignore_reason: checking existance | ||
| 'en': str(row['title_en'], # type: ignore | ||
| 'utf-8') if row['title_en'] else '', |
There was a problem hiding this comment.
| # type_ignore_reason: checking existance | |
| 'en': str(row['title_en'], # type: ignore | |
| 'utf-8') if row['title_en'] else '', | |
| 'en': row['title_en'], |
Should work fine now that .encode('utf-8') has been removed above?
For the type checks we could define the set of keys with a class Suggested(TypedDict): ... and then cast the row? There must be a clean way of doing DictReader dict key checks with pyright.
There was a problem hiding this comment.
Yeah, I personally have never used this command. Nor had to do anything with suggested datasets to this date. So didn't really put much effort into this one.
I have cleaned up this encoding stuff now. We are on Python3 now so hopefully it is all just fine. We will find out if we ever actually have to use this command I guess.
ckanext/canada/helpers.py
Outdated
| @@ -505,19 +508,19 @@ def json_loads(value: str) -> Dict[str, Any]: | |||
|
|
|||
|
|
|||
| def get_datapreview(res_id: str) -> str: | |||
| dsq_results = ckan.logic.get_action('datastore_search')( | |||
| {}, {'resource_id': res_id, 'limit': 100}) | |||
| dsq_results = t.get_action('datastore_search')( | |||
There was a problem hiding this comment.
we really shouldn't be using t as a global variable for ckan.toolkit. One-letter variables are almost always short-lived loop variables and the like.
There is precedence for using tk for the toolkit module, so we could change it to that.
There was a problem hiding this comment.
I personally hate the use of t alias for toolkit so thank god hahaha. I also don't like tk, but that is just my personaly preference because I am weird. (* continues to use underscored named variables in python)
I will just do toolkit
@wardi in the topic of toolkit do we even need this anymore: https://github.com/ckan/ckantoolkit
There was a problem hiding this comment.
Okay, t fixed up here and just used imports from ckan.plugins.toolkit
So just the big 'ol question about the ckantoolkit repo
- Various changes to update code to CKAN and python standards.
- Updated recombinant branch to `master`
No description provided.